Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(common): cleanup error handling for KmxFileReader and CompilerOptions as shared type #8951

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion common/web/types/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"dependencies": {
"@keymanapp/keyman-version": "*",
"ajv": "^8.11.0",
"restructure": "git+https://github.com/keymanapp/dependency-restructure.git#49d129cf0916d082a7278bb09296fb89cecfcc50",
"restructure": "git+https://github.com/keymanapp/dependency-restructure.git#7a188a1e26f8f36a175d95b67ffece8702363dfc",
"semver": "^7.3.7",
"xml2js": "git+https://github.com/keymanapp/dependency-node-xml2js#535fe732dc408d697e0f847c944cc45f0baf0829"
},
Expand Down
102 changes: 58 additions & 44 deletions common/web/types/src/kmx/kmx-file-reader.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { KMXFile, BUILDER_COMP_KEYBOARD, KEYBOARD, STORE, GROUP, KEY } from "./kmx.js";
import * as r from 'restructure';
import { KeymanTypesError } from "../util/errors.js";

export class KmxFileReaderError extends KeymanTypesError {

}

export class KmxFileReader {
private readonly rString = new r.String(null, 'utf16le');
Expand All @@ -13,7 +18,17 @@ export class KmxFileReader {
return this.rString.fromBuffer(source.slice(offset));
}

private processSystemStore(store: STORE, result: KEYBOARD) {
private isValidCodeUse(s: string, keyboard: KEYBOARD): boolean {
return (
s.length == 3 &&
s.charCodeAt(0) == KMXFile.UC_SENTINEL &&
s.charCodeAt(1) == KMXFile.CODE_USE &&
s.charCodeAt(2) > 0 &&
s.charCodeAt(2) <= keyboard.groups.length
);
}

private processSystemStore(store: STORE, result: KEYBOARD): void {
switch(store.dwSystemID) {
case KMXFile.TSS_MNEMONIC:
result.isMnemonic = store.dpString == '1';
Expand All @@ -22,34 +37,26 @@ export class KmxFileReader {
result.keyboardVersion = store.dpString;
break;
case KMXFile.TSS_BEGIN_NEWCONTEXT:
if(store.dpString.length == 3 && store.dpString.charCodeAt(0) == 0xFFFF && store.dpString.charCodeAt(1) == KMXFile.CODE_USE) {
result.startGroup.newContext = store.dpString.charCodeAt(2) - 1;
}
else {
// TODO: error
return false;
if(!this.isValidCodeUse(store.dpString, result)) {
throw new KmxFileReaderError(`Invalid TSS_BEGIN_NEWCONTEXT system store`);
}
result.startGroup.newContext = store.dpString.charCodeAt(2) - 1;
break;
case KMXFile.TSS_BEGIN_POSTKEYSTROKE:
if(store.dpString.length == 3 && store.dpString.charCodeAt(0) == 0xFFFF && store.dpString.charCodeAt(1) == KMXFile.CODE_USE) {
result.startGroup.postKeystroke = store.dpString.charCodeAt(2) - 1;
}
else {
// TODO: error
return false;
if(!this.isValidCodeUse(store.dpString, result)) {
throw new KmxFileReaderError(`Invalid TSS_BEGIN_POSTKEYSTROKE system store`);
}
result.startGroup.postKeystroke = store.dpString.charCodeAt(2) - 1;
break;
}
return true;
}

public read(source: Uint8Array): KEYBOARD {
let binaryKeyboard: BUILDER_COMP_KEYBOARD;
let kmx = new KMXFile();
binaryKeyboard = kmx.COMP_KEYBOARD.fromBuffer(source);
if(binaryKeyboard.dwIdentifier != KMXFile.FILEID_COMPILED) {
// TODO: error
return null;
throw new KmxFileReaderError(`Not a .kmx file: header does not contain FILEID_COMPILED`);
}

let result = new KEYBOARD();
Expand All @@ -59,32 +66,39 @@ export class KmxFileReader {
result.startGroup = {
ansi: binaryKeyboard.StartGroup_ANSI == 0xFFFFFFFF ? -1 : binaryKeyboard.StartGroup_ANSI,
unicode: binaryKeyboard.StartGroup_Unicode == 0xFFFFFFFF ? -1 : binaryKeyboard.StartGroup_Unicode,
newContext: -1, //TODO
postKeystroke: -1 // TODO
newContext: -1,
postKeystroke: -1
Comment on lines +69 to +70
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... what here got moved from "TODO" to "to-done"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TODO items were done a while ago and I forgot to remove the comments when I fixed them up

}

// Informative data

result.keyboardVersion = '';
result.isMnemonic = false;

let offset = binaryKeyboard.dpStoreArray;
for(let i = 0; i < binaryKeyboard.cxStoreArray; i++) {
let binaryStore = kmx.COMP_STORE.fromBuffer(source.slice(offset));
let store = new STORE();
store.dwSystemID = binaryStore.dwSystemID;
store.dpName = this.readString(source, binaryStore.dpName);
store.dpString = this.readString(source, binaryStore.dpString);
result.stores.push(store);
this.readStores(binaryKeyboard, kmx, source, result);
this.readGroupsAndRules(binaryKeyboard, kmx, source, result);

if(!this.processSystemStore(store, result)) {
return null;
}
// Process system stores once we have all stores and groups loaded
for(let store of result.stores) {
this.processSystemStore(store, result);
}

offset += KMXFile.COMP_STORE_SIZE;
// TODO: KMXPlusFile

// Validate startGroup offsets
let gp: keyof KEYBOARD['startGroup'];
for(gp in result.startGroup) {
if(result.startGroup[gp] < -1 || result.startGroup[gp] >= result.groups.length) {
throw new KmxFileReaderError(`Invalid begin group reference`);
}
}

offset = binaryKeyboard.dpGroupArray;
for(let i = 0; i < binaryKeyboard.cxGroupArray; i++) {
return result;
}

private readGroupsAndRules(binaryKeyboard: BUILDER_COMP_KEYBOARD, kmx: KMXFile, source: Uint8Array, result: KEYBOARD) {
let offset = binaryKeyboard.dpGroupArray;
for (let i = 0; i < binaryKeyboard.cxGroupArray; i++) {
let binaryGroup = kmx.COMP_GROUP.fromBuffer(source.slice(offset));
let group = new GROUP();
group.dpMatch = this.readString(source, binaryGroup.dpMatch);
Expand All @@ -94,7 +108,7 @@ export class KmxFileReader {
group.keys = [];

let keyOffset = binaryGroup.dpKeyArray;
for(let j = 0; j < binaryGroup.cxKeyArray; j++) {
for (let j = 0; j < binaryGroup.cxKeyArray; j++) {
let binaryKey = kmx.COMP_KEY.fromBuffer(source.slice(keyOffset));
let key = new KEY();
key.Key = binaryKey.Key;
Expand All @@ -109,18 +123,18 @@ export class KmxFileReader {
result.groups.push(group);
offset += KMXFile.COMP_GROUP_SIZE;
}
}

// TODO: KMXPlusFile

// Validate startGroup offsets
let gp: keyof KEYBOARD['startGroup'];
for(gp in result.startGroup) {
if(result.startGroup[gp] < -1 || result.startGroup[gp] >= result.groups.length) {
// TODO: error
return null;
}
private readStores(binaryKeyboard: BUILDER_COMP_KEYBOARD, kmx: KMXFile, source: Uint8Array, result: KEYBOARD): void {
let offset = binaryKeyboard.dpStoreArray;
for (let i = 0; i < binaryKeyboard.cxStoreArray; i++) {
let binaryStore = kmx.COMP_STORE.fromBuffer(source.slice(offset));
let store = new STORE();
store.dwSystemID = binaryStore.dwSystemID;
store.dpName = this.readString(source, binaryStore.dpName);
store.dpString = this.readString(source, binaryStore.dpString);
result.stores.push(store);
offset += KMXFile.COMP_STORE_SIZE;
}

return result;
}
};
4 changes: 2 additions & 2 deletions common/web/types/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
export * as KMX from './kmx/kmx.js';
export * as KMXPlus from './kmx/kmx-plus.js';
export { default as KMXBuilder } from './kmx/kmx-builder.js';
export { KmxFileReader } from './kmx/kmx-file-reader.js';
export { KmxFileReader, KmxFileReaderError } from './kmx/kmx-file-reader.js';

export * as VisualKeyboard from './kvk/visual-keyboard.js';
export { default as KMXPlusBuilder} from './kmx/kmx-plus-builder/kmx-plus-builder.js';
Expand All @@ -18,7 +18,7 @@ export { LDMLKeyboardXMLSourceFileReader, LDMLKeyboardXMLSourceFileReaderOptions

export * as Constants from './consts/virtual-key-constants.js';

export { CompilerCallbacks, CompilerSchema, CompilerEvent, CompilerErrorNamespace, CompilerErrorSeverity, CompilerPathCallbacks, CompilerFileSystemCallbacks, CompilerMessageSpec, compilerErrorSeverityName, compilerExceptionToString, compilerErrorFormatCode } from './util/compiler-interfaces.js';
export { CompilerCallbacks, CompilerOptions, CompilerSchema, CompilerEvent, CompilerErrorNamespace, CompilerErrorSeverity, CompilerPathCallbacks, CompilerFileSystemCallbacks, CompilerMessageSpec, compilerErrorSeverityName, compilerExceptionToString, compilerErrorFormatCode } from './util/compiler-interfaces.js';
export { CommonTypesMessages } from './util/common-events.js';

export * as TouchLayout from './keyman-touch-layout/keyman-touch-layout-file.js';
Expand Down
10 changes: 10 additions & 0 deletions common/web/types/src/util/compiler-interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,16 @@ export interface CompilerCallbacks {
debug(msg: string): void;
};

/**
* Abstract interface for compiler options
*/
export interface CompilerOptions {
shouldAddCompilerVersion?: boolean;
saveDebug?: boolean;
compilerWarningsAsErrors?: boolean;
warnDeprecatedCode?: boolean;
};

/**
* Convenience function for constructing CompilerEvents
* @param code
Expand Down
6 changes: 6 additions & 0 deletions common/web/types/src/util/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* Base class for all common/web/types errors thrown
*/
export class KeymanTypesError extends Error {

}
14 changes: 5 additions & 9 deletions developer/src/kmc-kmn/src/compiler/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ TODO: implement additional interfaces:
*/

// TODO: rename wasm-host?
import { CompilerCallbacks, CompilerEvent, KvkFileWriter, KvksFileReader } from '@keymanapp/common-types';
import { CompilerCallbacks, CompilerEvent, CompilerOptions, KvkFileWriter, KvksFileReader } from '@keymanapp/common-types';
import loadWasmHost from '../import/kmcmplib/wasm-host.js';
import { CompilerMessages, mapErrorFromKmcmplib } from './messages.js';

Expand All @@ -21,15 +21,11 @@ export interface CompilerResult {
js?: CompilerResultFile;
};

export interface CompilerOptions {
shouldAddCompilerVersion?: boolean;
saveDebug?: boolean;
compilerWarningsAsErrors?: boolean;
warnDeprecatedCode?: boolean;
export interface KmnCompilerOptions extends CompilerOptions {
target?: 'kmx' | 'js';
};

const baseOptions: CompilerOptions = {
const baseOptions: KmnCompilerOptions = {
shouldAddCompilerVersion: true,
saveDebug: true,
compilerWarningsAsErrors: false,
Expand Down Expand Up @@ -90,7 +86,7 @@ export class KmnCompiler {
return true;
}

public run(infile: string, outfile: string, options?: CompilerOptions): boolean {
public run(infile: string, outfile: string, options?: KmnCompilerOptions): boolean {
let result = this.runCompiler(infile, outfile, options);
if(result) {
if(result.kmx) {
Expand Down Expand Up @@ -146,7 +142,7 @@ export class KmnCompiler {
return 1;
}

public runCompiler(infile: string, outfile: string, options: CompilerOptions): CompilerResult {
public runCompiler(infile: string, outfile: string, options: KmnCompilerOptions): CompilerResult {
if(!this.verifyInitialized()) {
/* c8 ignore next 2 */
return null;
Expand Down
3 changes: 2 additions & 1 deletion developer/src/kmc-kmw/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@
},
"dependencies": {
"@keymanapp/common-types": "*",
"@keymanapp/keyman-version": "*",
"@keymanapp/kmc-kmn": "*",
"ajv": "^8.11.0",
"restructure": "git+https://github.com/keymanapp/dependency-restructure.git#49d129cf0916d082a7278bb09296fb89cecfcc50",
"restructure": "git+https://github.com/keymanapp/dependency-restructure.git#7a188a1e26f8f36a175d95b67ffece8702363dfc",
"semver": "^7.3.7",
"xml2js": "git+https://github.com/keymanapp/dependency-node-xml2js#535fe732dc408d697e0f847c944cc45f0baf0829"
},
Expand Down
14 changes: 0 additions & 14 deletions developer/src/kmc-kmw/src/compiler/callbacks.ts

This file was deleted.

3 changes: 1 addition & 2 deletions developer/src/kmc-kmw/src/compiler/compiler-globals.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { KMX, CompilerCallbacks } from "@keymanapp/common-types";
import CompilerOptions from "./compiler-options.js";
import { KMX, CompilerCallbacks, CompilerOptions } from "@keymanapp/common-types";

export let FTabStop: string;
export let nl: string;
Expand Down
15 changes: 0 additions & 15 deletions developer/src/kmc-kmw/src/compiler/compiler-options.ts

This file was deleted.

8 changes: 4 additions & 4 deletions developer/src/kmc-kmw/src/compiler/javascript-strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export function JavaScript_Name(i: number, pwszName: string, KeepNameForPersiste
let FChanged: boolean = false;
let p = pwszName;

if((pwszName == null || pwszName == undefined || pwszName == '') || (!options.debug && !KeepNameForPersistentStorage)) { // I3659 // I3681
if((pwszName == null || pwszName == undefined || pwszName == '') || (!options.saveDebug && !KeepNameForPersistentStorage)) { // I3659 // I3681
return i.toString(10); // for uniqueness
}
else {
Expand Down Expand Up @@ -138,7 +138,7 @@ function JavaScript_Rule(FTabStops: string, FElse: string, fk: KMX.KEYBOARD, fgp
let predicate: string = '1', linecomment: string = '', FIndent: string;
let result = '';

if(fkp.Line > 0 && options.debug) { // I4384
if(fkp.Line > 0 && options.saveDebug) { // I4384
linecomment = ' // Line '+fkp.Line.toString(); // I4373
}

Expand Down Expand Up @@ -318,7 +318,7 @@ export function JavaScript_Shift(fkp: KMX.KEY, FMnemonic: boolean): number {
* '16656'
*/
export function JavaScript_ShiftAsString(fkp: KMX.KEY, FMnemonic: boolean): string {
if(!options.debug) {
if(!options.saveDebug) {
return JavaScript_Shift(fkp, FMnemonic).toString();
}
return ' '+FormatModifierAsBitflags(JavaScript_Shift(fkp, FMnemonic));
Expand Down Expand Up @@ -722,7 +722,7 @@ export function JavaScript_Key(fkp: KMX.KEY, FMnemonic: boolean): number {
/// @return string representation of the key value, e.g. 'keyCodes.K_A /* 0x41 */' or '65'
///
export function JavaScript_KeyAsString(fkp: KMX.KEY, FMnemonic: boolean): string {
if(options.debug) {
if(options.saveDebug) {
return ' '+FormatKeyAsString(JavaScript_Key(fkp, FMnemonic));
} else {
return JavaScript_Key(fkp, FMnemonic).toString();
Expand Down
Loading