diff --git a/developer/src/kmc-ldml/src/compiler/keys.ts b/developer/src/kmc-ldml/src/compiler/keys.ts index 7b79a3be83..f1416cb6d0 100644 --- a/developer/src/kmc-ldml/src/compiler/keys.ts +++ b/developer/src/kmc-ldml/src/compiler/keys.ts @@ -73,7 +73,7 @@ export class KeysCompiler extends SectionCompiler { this.keyboard3.forms?.form?.forEach((form) => { if (!LDMLKeyboard.ImportStatus.isImpliedImport(form)) { // If it's not an implied import, give a warning. - this.callbacks.reportMessage(LdmlCompilerMessages.Warn_CustomForm({ id: form.id })); + this.callbacks.reportMessage(LdmlCompilerMessages.Warn_CustomForm(form)); } }); @@ -100,7 +100,7 @@ export class KeysCompiler extends SectionCompiler { if (!flickHash.has(flickId)) { valid = false; this.callbacks.reportMessage( - LdmlCompilerMessages.Error_MissingFlicks({ flickId, id: keyId }) + LdmlCompilerMessages.Error_MissingFlicks(key) ); } } @@ -109,10 +109,13 @@ export class KeysCompiler extends SectionCompiler { for(const [gestureKeyId, attrs] of gestureKeys.entries()) { const gestureKey = keyBag.get(gestureKeyId); if (gestureKey == null) { - // TODO-LDML: could keep track of already missing keys so we don't warn multiple times on gesture keys + // We don't keep track of already missing keys - might warn multiple times on gesture keys, + // however we leave it to the caller to handle eliding duplicate messages valid = false; this.callbacks.reportMessage( - LdmlCompilerMessages.Error_GestureKeyNotFoundInKeyBag({keyId: gestureKeyId, parentKeyId: keyId, attribute: attrs.join(',')}) + LdmlCompilerMessages.Error_GestureKeyNotFoundInKeyBag({ + keyId: gestureKeyId, parentKeyId: keyId, attribute: attrs.join(',') + }, key) ); } else { usedKeys.add(gestureKeyId); @@ -132,9 +135,14 @@ export class KeysCompiler extends SectionCompiler { if (hardwareLayers.length >= 1) { // validate all errors for (const layers of hardwareLayers) { - for (const layer of layers.layer) { - valid = - this.validateHardwareLayerForKmap(layers.formId, layer, keyBag) && valid; // note: always validate even if previously invalid results found + const keymap = this.validateAndGetKeymapFromLayers(layers); + if (!keymap) { + valid = false; + } else { + for (const layer of layers.layer) { + valid = + this.validateHardwareLayerForKmap(keymap, layers, layer, keyBag) && valid; // note: always validate even if previously invalid results found + } } } // TODO-LDML: } else { touch? @@ -143,6 +151,28 @@ export class KeysCompiler extends SectionCompiler { return valid; } + private validateAndGetKeymapFromLayers(layers: LDMLKeyboard.LKLayers) : Constants.KeyMap { + const { formId } = layers; + const badScans = new Set(); + const form = this.getForm(formId); + if (!form) { + this.callbacks.reportMessage( + LdmlCompilerMessages.Error_InvalidHardware(layers) + ); + return null; + } + const keymap = KeysCompiler.getKeymapFromScancodes(form, badScans); + if (!keymap) { + } else if (badScans.size !== 0) { + const codes = Array.from(badScans.values()).map(n => Number(n).toString(16)).sort(); + this.callbacks.reportMessage( + LdmlCompilerMessages.Error_InvalidScanCode({ codes }, form) + ); + return null; + } + return keymap; + } + static addKeysFromFlicks(usedFlicks: Set, flickHash: Map, usedKeys: Set) { for (const flickId of usedFlicks.values()) { const flick = flickHash.get(flickId); @@ -390,14 +420,22 @@ export class KeysCompiler extends SectionCompiler { } public static getKeymapFromForms(forms: LDMLKeyboard.LKForm[], hardware: string, badScans?: Set): Constants.KeyMap { - // seach in reverse form because of overrides - const ldmlForm = [...forms].reverse().find((f) => f.id === hardware); + const ldmlForm = KeysCompiler.findForm(forms, hardware); if (!ldmlForm) { return undefined; } return KeysCompiler.getKeymapFromScancodes(ldmlForm, badScans); } + private getForm(hardware: string) { + return KeysCompiler.findForm(this.keyboard3?.forms.form, hardware); + } + + public static findForm(forms: LDMLKeyboard.LKForm[], hardware: string) { + // seach in reverse form because of overrides + return [...forms].reverse().find((f) => f.id === hardware); + } + public static getKeymapFromScancodes(ldmlForm: LDMLKeyboard.LKForm, badScans?: Set) { const { scanCodes } = ldmlForm; const ldmlScan = scanCodes.map(o => o.codes.split(" ").map(n => Number.parseInt(n, 16))); @@ -413,7 +451,8 @@ export class KeysCompiler extends SectionCompiler { * @returns true if valid */ private validateHardwareLayerForKmap( - hardware: string, + keymap: Constants.KeyMap, + layers: LDMLKeyboard.LKLayers, layer: LDMLKeyboard.LKLayer, keyHash: Map ): boolean { @@ -422,28 +461,11 @@ export class KeysCompiler extends SectionCompiler { const { modifiers } = layer; if (!validModifier(modifiers)) { this.callbacks.reportMessage( - LdmlCompilerMessages.Error_InvalidModifier({ modifiers, layer: layer.id }) + LdmlCompilerMessages.Error_InvalidModifier(layer) ); valid = false; } - const badScans = new Set(); - const keymap = this.getKeymapFromForm(hardware, badScans); - if (!keymap) { - this.callbacks.reportMessage( - LdmlCompilerMessages.Error_InvalidHardware({ formId: hardware }) - ); - valid = false; - return valid; - } else if (badScans.size !== 0) { - const codes = Array.from(badScans.values()).map(n => Number(n).toString(16)).sort(); - this.callbacks.reportMessage( - LdmlCompilerMessages.Error_InvalidScanCode({ form: hardware, codes }) - ); - valid = false; - return valid; - } - if (layer.row.length > keymap.length) { this.callbacks.reportMessage( LdmlCompilerMessages.Error_HardwareLayerHasTooManyRows(layer) @@ -452,15 +474,16 @@ export class KeysCompiler extends SectionCompiler { } for (let y = 0; y < layer.row.length && y < keymap.length; y++) { - const keys = layer.row[y].keys.split(" "); + const row = layer.row[y]; + const keys = row.keys.split(" "); if (keys.length > keymap[y].length) { this.callbacks.reportMessage( LdmlCompilerMessages.Error_RowOnHardwareLayerHasTooManyKeys({ row: y + 1, - hardware, + hardware: layers.formId, modifiers, - }) + }, row) ); valid = false; } @@ -478,7 +501,7 @@ export class KeysCompiler extends SectionCompiler { row: y + 1, layer: layer.id, form: "hardware", - }) + }, row) ); valid = false; continue; diff --git a/developer/src/kmc-ldml/src/compiler/layr.ts b/developer/src/kmc-ldml/src/compiler/layr.ts index 6c945a1864..9b1bd824ad 100644 --- a/developer/src/kmc-ldml/src/compiler/layr.ts +++ b/developer/src/kmc-ldml/src/compiler/layr.ts @@ -37,10 +37,10 @@ export class LayrCompiler extends SectionCompiler { } } layers.layer.forEach((layer) => { - const { modifiers, id } = layer; + const { modifiers } = layer; totalLayerCount++; if (!validModifier(modifiers)) { - this.callbacks.reportMessage(LdmlCompilerMessages.Error_InvalidModifier({ modifiers, layer: id || '' })); + this.callbacks.reportMessage(LdmlCompilerMessages.Error_InvalidModifier(layer)); valid = false; } }); @@ -65,7 +65,8 @@ export class LayrCompiler extends SectionCompiler { const erow: LayrRow = { keys: row.keys.trim().split(/[ \t]+/).map((id) => sections.strs.allocString(id)), }; - return erow; + // include linenumber info for row + return SectionCompiler.copySymbols(erow, row); }); const mods = translateLayerAttrToModifier(layer); // push a layer entry for each modifier set diff --git a/developer/src/kmc-ldml/src/compiler/ldml-compiler-messages.ts b/developer/src/kmc-ldml/src/compiler/ldml-compiler-messages.ts index 28c5b52e1f..40b9160382 100644 --- a/developer/src/kmc-ldml/src/compiler/ldml-compiler-messages.ts +++ b/developer/src/kmc-ldml/src/compiler/ldml-compiler-messages.ts @@ -1,5 +1,6 @@ -import { util } from "@keymanapp/common-types"; +import { KMXPlus, util } from "@keymanapp/common-types"; import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerMessageSpec as m, CompilerMessageDef as def, XML_FILENAME_SYMBOL, CompilerEvent, KeymanXMLReader } from '@keymanapp/developer-utils'; +import { LDMLKeyboard } from '@keymanapp/developer-utils'; // const SevInfo = CompilerErrorSeverity.Info | CompilerErrorNamespace.LdmlKeyboardCompiler; const SevHint = CompilerErrorSeverity.Hint | CompilerErrorNamespace.LdmlKeyboardCompiler; const SevWarn = CompilerErrorSeverity.Warn | CompilerErrorNamespace.LdmlKeyboardCompiler; @@ -53,11 +54,18 @@ export class LdmlCompilerMessages { ); static ERROR_RowOnHardwareLayerHasTooManyKeys = SevError | 0x0004; - static Error_RowOnHardwareLayerHasTooManyKeys = (o:{row: number, hardware: string, modifiers: string}) => m(this.ERROR_RowOnHardwareLayerHasTooManyKeys, `Row #${def(o.row)} on 'hardware' ${def(o.hardware)} layer for modifier ${o.modifiers || 'none'} has too many keys`); + static Error_RowOnHardwareLayerHasTooManyKeys = (o: { row: number, hardware: string, modifiers: string }, x: LDMLKeyboard.LKRow) => mx( + this.ERROR_RowOnHardwareLayerHasTooManyKeys, + `Row #${def(o.row)} on 'hardware' ${def(o.hardware)} layer for modifier ${o.modifiers || 'none'} has too many keys`, + x, + ); static ERROR_KeyNotFoundInKeyBag = SevError | 0x0005; - static Error_KeyNotFoundInKeyBag = (o:{keyId: string, col: number, row: number, layer: string, form: string}) => - m(this.ERROR_KeyNotFoundInKeyBag, `Key '${def(o.keyId)}' in position #${def(o.col)} on row #${def(o.row)} of layer ${def(o.layer)}, form '${def(o.form)}' not found in key bag`); + static Error_KeyNotFoundInKeyBag = (o: { keyId: string, col: number, row: number, layer: string, form: string }, x: LDMLKeyboard.LKRow | KMXPlus.LayrRow) => mx( + this.ERROR_KeyNotFoundInKeyBag, + `Key '${def(o.keyId)}' in position #${def(o.col)} on row #${def(o.row)} of layer ${def(o.layer)}, form '${def(o.form)}' not found in key bag`, + x, + ); static HINT_OneOrMoreRepeatedLocales = SevHint | 0x0006; static Hint_OneOrMoreRepeatedLocales = () => @@ -72,16 +80,26 @@ export class LdmlCompilerMessages { m(this.HINT_LocaleIsNotMinimalAndClean, `Locale '${def(o.sourceLocale)}' is not minimal or correctly formatted and should be '${def(o.locale)}'`); static ERROR_InvalidScanCode = SevError | 0x0009; - static Error_InvalidScanCode = (o:{form?: string, codes?: string[]}) => - m(this.ERROR_InvalidScanCode, `Form '${def(o.form)}' has invalid/unknown scancodes '${def(o.codes?.join(' '))}'`); + static Error_InvalidScanCode = (o:{codes?: string[]}, x: LDMLKeyboard.LKForm) => mx( + this.ERROR_InvalidScanCode, + `Form '${def(x?.id)}' has invalid/unknown scancodes '${def(o.codes?.join(' '))}'`, + x, + ); static WARN_CustomForm = SevWarn | 0x000A; - static Warn_CustomForm = (o:{id: string}) => - m(this.WARN_CustomForm, `Custom
element. Key layout may not be as expected.`); + static Warn_CustomForm = (o: LDMLKeyboard.LKForm) => mx( + this.WARN_CustomForm, + `Custom element. Key layout may not be as expected.`, + o, + ); static ERROR_GestureKeyNotFoundInKeyBag = SevError | 0x000B; - static Error_GestureKeyNotFoundInKeyBag = (o:{keyId: string, parentKeyId: string, attribute: string}) => - m(this.ERROR_GestureKeyNotFoundInKeyBag, `Key '${def(o.keyId)}' not found in key bag, referenced from other '${def(o.parentKeyId)}' in ${def(o.attribute)}`); + static Error_GestureKeyNotFoundInKeyBag = (o:{keyId: string, parentKeyId: string, attribute: string}, x: LDMLKeyboard.LKKey) => + mx( + this.ERROR_GestureKeyNotFoundInKeyBag, + `Key '${def(o.keyId)}' not found in key bag, referenced from other '${def(o.parentKeyId)}' in ${def(o.attribute)}`, + x, + ); static HINT_NoDisplayForMarker = SevHint | 0x000C; static Hint_NoDisplayForMarker = (o: { id: string }) => @@ -138,8 +156,11 @@ export class LdmlCompilerMessages { `layers formId=${def(o.formId)}: Can only have one non-'touch' element`); static ERROR_InvalidHardware = SevError | 0x0013; - static Error_InvalidHardware = (o:{formId: string}) => m(this.ERROR_InvalidHardware, - `layers has invalid value formId=${def(o.formId)}`); + static Error_InvalidHardware = (o:LDMLKeyboard.LKLayers) => mx( + this.ERROR_InvalidHardware, + `layers has invalid value formId=${def(o.formId)}`, + o, + ); private static layerIdOrEmpty(layer : string) { if (layer) { @@ -150,12 +171,18 @@ export class LdmlCompilerMessages { } static ERROR_InvalidModifier = SevError | 0x0014; - static Error_InvalidModifier = (o:{layer: string, modifiers: string}) => m(this.ERROR_InvalidModifier, - `layer has invalid modifiers='${def(o.modifiers)}'` + LdmlCompilerMessages.layerIdOrEmpty(o.layer)); + static Error_InvalidModifier = (o:LDMLKeyboard.LKLayer) => mx( + this.ERROR_InvalidModifier, + `layer has invalid modifiers='${def(o.modifiers)}'` + LdmlCompilerMessages.layerIdOrEmpty(o.id), + o, + ); static ERROR_MissingFlicks = SevError | 0x0015; - static Error_MissingFlicks = (o:{flickId: string, id: string}) => m(this.ERROR_MissingFlicks, - `key id=${def(o.id)} refers to missing flickId=${def(o.flickId)}`); + static Error_MissingFlicks = (o: LDMLKeyboard.LKKey) => mx( + this.ERROR_MissingFlicks, + `key id=${def(o.id)} refers to missing flickId=${def(o.flickId)}`, + o, + ); static ERROR_DuplicateVariable = SevError | 0x0016; static Error_DuplicateVariable = (o:{ids: string}) => m(this.ERROR_DuplicateVariable, @@ -316,8 +343,14 @@ export class LdmlCompilerMessages { static offset(event: CompilerEvent, x?: any): CompilerEvent { if(x) { const metadata = KeymanXMLReader.getMetaData(x) || {}; - event.offset = metadata?.startIndex; - event.filename = event.filename || metadata[XML_FILENAME_SYMBOL]; + const offset = metadata?.startIndex; + if (offset) { + event.offset = offset; + } + const filename = event.filename || metadata[XML_FILENAME_SYMBOL]; + if (filename) { + event.filename = filename; + } } return event; } diff --git a/developer/src/kmc-ldml/src/compiler/visual-keyboard-compiler.ts b/developer/src/kmc-ldml/src/compiler/visual-keyboard-compiler.ts index 56431b63c3..480ac5add5 100644 --- a/developer/src/kmc-ldml/src/compiler/visual-keyboard-compiler.ts +++ b/developer/src/kmc-ldml/src/compiler/visual-keyboard-compiler.ts @@ -117,7 +117,9 @@ export class LdmlKeyboardVisualKeyboardCompiler { if (!keydef || !kmap || text === null) { this.callbacks.reportMessage( - LdmlCompilerMessages.Error_KeyNotFoundInKeyBag({ keyId: key.value, layer: layerId, row: y, col: x, form: hardware }) + LdmlCompilerMessages.Error_KeyNotFoundInKeyBag({ + keyId: key.value, layer: layerId, row: y, col: x, form: hardware + }, row) ); result = false; } else { diff --git a/developer/src/kmc-ldml/test/helpers/index.ts b/developer/src/kmc-ldml/test/helpers/index.ts index 8cda5f5d4e..7d714bb2b6 100644 --- a/developer/src/kmc-ldml/test/helpers/index.ts +++ b/developer/src/kmc-ldml/test/helpers/index.ts @@ -8,7 +8,7 @@ import * as path from 'path'; import { fileURLToPath } from 'url'; import { SectionCompiler, SectionCompilerNew } from '../../src/compiler/section-compiler.js'; import { util, KMXPlus, LdmlKeyboardTypes } from '@keymanapp/common-types'; -import { CompilerEvent, compilerEventFormat, CompilerCallbacks, LDMLKeyboardXMLSourceFileReader, LDMLKeyboardTestDataXMLSourceFile, LDMLKeyboard, KeymanXMLMetadata, KeymanXMLReader } from "@keymanapp/developer-utils"; +import { CompilerEvent, compilerEventFormat, CompilerCallbacks, LDMLKeyboardXMLSourceFileReader, LDMLKeyboardTestDataXMLSourceFile, LDMLKeyboard, KeymanXMLMetadata, KeymanXMLReader, CompilerError } from "@keymanapp/developer-utils"; import { LdmlKeyboardCompiler } from '../../src/main.js'; // make sure main.js compiles import { assert } from 'chai'; import { KMXPlusMetadataCompiler } from '../../src/compiler/metadata-compiler.js'; @@ -252,6 +252,11 @@ export interface CompilationCase { * Optional, if true, postValidate() must return false. (must be != postValidate()) */ postValidateFail?: boolean; + /** + * retain offset (line number) information. otherwise, scrub it to reduce testing noise. + * only tests that specifically are checking offsets will set this to true + */ + retainOffsetInMessages?: boolean; } /** @@ -275,26 +280,39 @@ export function testCompilationCases(compiler: SectionCompilerNew, cases : Compi return; } const section = await loadSectionFixture(compiler, testcase.subpath, callbacks, testcase.dependencies || dependencies); - const testcaseErrors = matchCompilerEventsOrBoolean(callbacks.messages, testcase.errors); - const testcaseWarnings = matchCompilerEvents(callbacks.messages, testcase.warnings); + let messagesToCheck = callbacks.messages; + // scrub offsets from messages to reduce churn in the test casws + if (!testcase.retainOffsetInMessages && callbacks.messages) { + messagesToCheck = callbacks.messages.map(m => { + const scrubbed = Object.assign({}, m); + // Turn this on once all messages have offsets, see messages.tests.ts + // if (!scrubbed.offset) { + // throw Error(`Error, no offset detected in message ${CompilerError.formatEvent(m)}`); + // } + delete scrubbed.offset; + return scrubbed; + }); + } + const testcaseErrors = matchCompilerEventsOrBoolean(messagesToCheck, testcase.errors); + const testcaseWarnings = matchCompilerEvents(messagesToCheck, testcase.warnings); // if we expected errors or warnings, show them if (testcaseErrors && testcaseErrors !== true) { - assert.includeDeepMembers(callbacks.messages, testcaseErrors, 'expected errors to be included'); + assert.includeDeepMembers(messagesToCheck, testcaseErrors, 'expected errors to be included'); } if (testcaseErrors && testcase.strictErrors) { - assert.sameDeepMembers(callbacks.messages, testcaseErrors, 'expected same errors to be included'); + assert.sameDeepMembers(messagesToCheck, testcaseErrors, 'expected same errors to be included'); } if (testcaseWarnings) { - assert.includeDeepMembers(callbacks.messages, testcaseWarnings, 'expected warnings to be included'); + assert.includeDeepMembers(messagesToCheck, testcaseWarnings, 'expected warnings to be included'); } else if (!expectFailure) { // no warnings, so expect zero messages - assert.sameDeepMembers(callbacks.messages, [], 'expected zero messages but got ' + callbacks.messages); + assert.sameDeepMembers(messagesToCheck, [], 'expected zero messages but got ' + callbacks.messages); } if (expectFailure) { assert.isNull(section, 'expected compilation result failure (null)'); } else { - assert.isNotNull(section, `failed with ${compilerEventFormat(callbacks.messages)}`); + assert.isNotNull(section, `failed with ${CompilerError.formatEvent(callbacks.messages)}`); } // run the user-supplied callback if any @@ -328,10 +346,15 @@ export function hex_str(s?: string) : string { return [...s].map(ch => dontEscape.test(ch) ? ch : util.escapeRegexChar(ch)).join(''); } -/** return an object simulating an XML object with a column number */ -export function withColumn(c: number) : KeymanXMLMetadata { +/** + * Return an object simulating an XML object with an offset number + * For use in calling message functions + * @param c number for the offset setting + * @param x if set, this object will be used as the base object instead of {} + */ +export function withOffset(c: number, x?: any) : KeymanXMLMetadata { // set metadata on an empty object - const o = {}; + const o = Object.assign({}, x); KeymanXMLReader.setMetaData(o, { startIndex: c }); diff --git a/developer/src/kmc-ldml/test/keys.tests.ts b/developer/src/kmc-ldml/test/keys.tests.ts index 3db4388a35..bc8e41fef7 100644 --- a/developer/src/kmc-ldml/test/keys.tests.ts +++ b/developer/src/kmc-ldml/test/keys.tests.ts @@ -1,7 +1,7 @@ import 'mocha'; import { assert } from 'chai'; import { KeysCompiler } from '../src/compiler/keys.js'; -import { assertCodePoints, compilerTestCallbacks, loadSectionFixture, testCompilationCases, withColumn } from './helpers/index.js'; +import { assertCodePoints, compilerTestCallbacks, loadSectionFixture, testCompilationCases, withOffset } from './helpers/index.js'; import { KMXPlus, Constants, LdmlKeyboardTypes } from '@keymanapp/common-types'; import { LdmlCompilerMessages } from '../src/compiler/ldml-compiler-messages.js'; import { constants } from '@keymanapp/ldml-keyboard-constants'; @@ -9,6 +9,7 @@ import { MetaCompiler } from '../src/compiler/meta.js'; const keysDependencies = [ ...BASIC_DEPENDENCIES, MetaCompiler ]; import Keys = KMXPlus.Keys; import { BASIC_DEPENDENCIES } from '../src/compiler/empty-compiler.js'; +import { LDMLKeyboard } from '@keymanapp/developer-utils'; const K = Constants.USVirtualKeyCodes; describe('keys', function () { @@ -311,7 +312,7 @@ describe('keys.kmap', function () { { subpath: 'sections/keys/invalid-bad-modifier.xml', errors: [ - LdmlCompilerMessages.Error_InvalidModifier({layer:'base',modifiers:'altR-shift'}), + LdmlCompilerMessages.Error_InvalidModifier({id:'base',modifiers:'altR-shift'}), ] }, { @@ -330,7 +331,11 @@ describe('keys.kmap', function () { // warning on custom form subpath: 'sections/layr/warn-custom-us-form.xml', warnings: [ - LdmlCompilerMessages.Warn_CustomForm({id: "us"}), + // most tests will want to leave retainOffsetInMessages: false to + // not require maintaining the offset here, which will break if + // the XML changes. + // However, it's worthwhile having at least one test that verifies in this way. + LdmlCompilerMessages.Warn_CustomForm(withOffset(367, {id: "us"}) as LDMLKeyboard.LKForm), ], callback: (sect, subpath, callbacks) => { const keys = sect as Keys; @@ -355,6 +360,9 @@ describe('keys.kmap', function () { }, ]); }, + // Note: Most tests will NOT want to set this. + // We set this here to test the test mechanism. + retainOffsetInMessages: true, }, { // warning on a custom unknown form - but no error! @@ -392,7 +400,7 @@ describe('keys.kmap', function () { LdmlCompilerMessages.Warn_CustomForm({id: "us"}), ], errors: [ - LdmlCompilerMessages.Error_InvalidScanCode({ form: "us", codes: ['ff'] }), + LdmlCompilerMessages.Error_InvalidScanCode({ codes: ['ff'] }, { id: 'us' }), ], }, { @@ -401,7 +409,7 @@ describe('keys.kmap', function () { LdmlCompilerMessages.Warn_CustomForm({id: "zzz"}), ], errors: [ - LdmlCompilerMessages.Error_InvalidScanCode({ form: "zzz", codes: ['ff'] }), + LdmlCompilerMessages.Error_InvalidScanCode({ codes: ['ff'] }, { id: "zzz" }), ], }, { @@ -442,7 +450,7 @@ describe('keys.kmap', function () { assert.isNull(keys); assert.equal(compilerTestCallbacks.messages.length, 1); - assert.deepEqual(compilerTestCallbacks.messages[0], LdmlCompilerMessages.Error_HardwareLayerHasTooManyRows(withColumn(276))); + assert.deepEqual(compilerTestCallbacks.messages[0], LdmlCompilerMessages.Error_HardwareLayerHasTooManyRows(withOffset(276))); }); it('should reject layouts with too many hardware keys', async function() { @@ -450,7 +458,7 @@ describe('keys.kmap', function () { assert.isNull(keys); assert.equal(compilerTestCallbacks.messages.length, 1); - assert.deepEqual(compilerTestCallbacks.messages[0], LdmlCompilerMessages.Error_RowOnHardwareLayerHasTooManyKeys({row: 1, hardware: 'us', modifiers: 'none'})); + assert.deepEqual(compilerTestCallbacks.messages[0], LdmlCompilerMessages.Error_RowOnHardwareLayerHasTooManyKeys({ row: 1, hardware: 'us', modifiers: 'none' }, withOffset(785) as LDMLKeyboard.LKRow)); }); it('should reject layouts with undefined keys', async function() { @@ -458,7 +466,7 @@ describe('keys.kmap', function () { assert.isNull(keys); assert.equal(compilerTestCallbacks.messages.length, 1); - assert.deepEqual(compilerTestCallbacks.messages[0], LdmlCompilerMessages.Error_KeyNotFoundInKeyBag({col: 1, form: 'hardware', keyId: 'foo', layer: 'base', row: 1})); + assert.deepEqual(compilerTestCallbacks.messages[0], LdmlCompilerMessages.Error_KeyNotFoundInKeyBag({col: 1, form: 'hardware', keyId: 'foo', layer: 'base', row: 1}, withOffset(271) as LDMLKeyboard.LKRow)); }); it('should reject layouts with invalid keys', async function() { const keys = await loadSectionFixture(KeysCompiler, 'sections/keys/invalid-key-missing-attrs.xml', compilerTestCallbacks, keysDependencies) as Keys; @@ -466,7 +474,7 @@ describe('keys.kmap', function () { assert.equal(compilerTestCallbacks.messages.length, 1); assert.deepEqual(compilerTestCallbacks.messages[0], LdmlCompilerMessages.Error_KeyMissingToGapOrSwitch( {keyId: 'Q'}, - withColumn(188) + withOffset(188) )); }); it('should accept layouts with gap/switch keys', async function() { diff --git a/developer/src/kmc-ldml/test/layr.tests.ts b/developer/src/kmc-ldml/test/layr.tests.ts index 8aeb604171..5d08dd621a 100644 --- a/developer/src/kmc-ldml/test/layr.tests.ts +++ b/developer/src/kmc-ldml/test/layr.tests.ts @@ -96,7 +96,7 @@ describe('layr', function () { subpath: 'sections/keys/invalid-bad-modifier.xml', errors: [ LdmlCompilerMessages.Error_InvalidModifier({ - layer: 'base', + id: 'base', modifiers: 'altR-shift' }), ], @@ -132,7 +132,7 @@ describe('layr', function () { { subpath: 'sections/layr/error-bogus-modifiers.xml', errors: [ - LdmlCompilerMessages.Error_InvalidModifier({ layer: '', modifiers: 'caps bogus'}), + LdmlCompilerMessages.Error_InvalidModifier({ id: '', modifiers: 'caps bogus'}), ] }, { diff --git a/developer/src/kmc-ldml/test/messages.tests.ts b/developer/src/kmc-ldml/test/messages.tests.ts index 041e1ed214..8b5593d569 100644 --- a/developer/src/kmc-ldml/test/messages.tests.ts +++ b/developer/src/kmc-ldml/test/messages.tests.ts @@ -1,10 +1,57 @@ import 'mocha'; +import {expect} from 'chai'; import { LdmlCompilerMessages } from '../src/compiler/ldml-compiler-messages.js'; import { verifyCompilerMessagesObject } from '@keymanapp/developer-test-helpers'; -import { CompilerErrorNamespace } from '@keymanapp/developer-utils'; +import { CompilerErrorNamespace, CompilerEvent } from '@keymanapp/developer-utils'; +import { withOffset } from './helpers/index.js'; describe('LdmlCompilerMessages', function () { it('should have a valid LdmlCompilerMessages object', function() { return verifyCompilerMessagesObject(LdmlCompilerMessages, CompilerErrorNamespace.LdmlKeyboardCompiler); }); + it('should have offset (line) reporting on all messages', function() { + const m = LdmlCompilerMessages as Record; + const keys = Object.keys(LdmlCompilerMessages); + /** all fns */ + let total = 0; + /** does not take line numbers */ + let noLines = 0; + /** takes line numbers */ + let lines = 0; + const fakeOffsetNumber = 1234; + const fakeOffsetObject = withOffset(fakeOffsetNumber); + for(const key of keys) { + if(typeof m[key] == 'function') { + total++; + const f = m[key] as Function; + // console.log(`${f.name}: ${f.length}`); + if (f.length === 0) { // Error_foo() + noLines++; + continue; + } + // now try to call it + let resp : CompilerEvent; + if (f.length === 1) { // Error_foo(x) + resp = f(fakeOffsetObject); + } else if(f.length >= 2) { // Error_foo(o, x) + resp = f({}, fakeOffsetObject); + } + expect(resp).to.be.ok; // should get an object one way or another + if(resp.offset) { + lines++; + expect(resp.offset).to.equal(fakeOffsetNumber, `Offset number round trip for error ${f.name} did not work, check the message function`); + } else { + // did not get a column number back + noLines++; + } + } + } + expect(lines).to.not.be.equal(0, `None of ${total} messages had offset reporting.`); + if (noLines > 0) { + // Once this goes to zero, make it an error if it goes up! + // Oh, and while you're here, once this is zero, uncomment the code in testCompilationCases + // that asserts that all messages are actually generated with an offset. + console.warn(`TODO-LDML (#10622) ${noLines}/${total} messages did not have detectable offset (line number) reporting.`); + } + }); });