From bc3a8ff36d19e95d8cf8c674c9cb448cdbde694f Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Fri, 30 May 2025 15:41:16 -0500 Subject: [PATCH 1/2] feat(developer): ldml: tran compiler: all relevant messages in ldml have line numbers! Fixes: #13937 --- .../src/compiler/ldml-compiler-messages.ts | 33 +++++++++++-------- developer/src/kmc-ldml/src/compiler/tran.ts | 22 ++++++------- developer/src/kmc-ldml/test/messages.tests.ts | 17 ++++------ 3 files changed, 37 insertions(+), 35 deletions(-) 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 6cbfa8e161..749d44ed67 100644 --- a/developer/src/kmc-ldml/src/compiler/ldml-compiler-messages.ts +++ b/developer/src/kmc-ldml/src/compiler/ldml-compiler-messages.ts @@ -85,6 +85,7 @@ export class LdmlCompilerMessages { `After minimization, one or more locales is repeated and has been removed`, ); + // This is the only allowed use of m() vs mx() in this file, all the others take context. static ERROR_InvalidFile = SevError | 0x0007; static Error_InvalidFile = (o:{errorText: string}) => m(this.ERROR_InvalidFile, `The source file has an invalid structure: ${def(o.errorText)}`); @@ -306,8 +307,10 @@ export class LdmlCompilerMessages { ); static ERROR_UnparseableReorderSet = SevError | 0x0028; - static Error_UnparseableReorderSet = (o: { from: string, set: string }) => - m(this.ERROR_UnparseableReorderSet, `Illegal UnicodeSet "${def(o.set)}" in reorder "${def(o.from)}`); + static Error_UnparseableReorderSet = (o: { from: string, set: string }, x?: ObjectWithMetadata) => mx( + this.ERROR_UnparseableReorderSet, x, + `Illegal UnicodeSet "${def(o.set)}" in reorder "${def(o.from)}`, + ); static ERROR_InvalidVariableIdentifier = SevError | 0x0029; static Error_InvalidVariableIdentifier = (o: { id: string }, x?: ObjectWithMetadata) => mx( @@ -354,16 +357,18 @@ export class LdmlCompilerMessages { // This is a bit of a catch-all and represents messages bubbling up from the underlying regex engine static ERROR_UnparseableTransformFrom = SevErrorTransform | 0x00; - static Error_UnparseableTransformFrom = (o: { from: string, message: string }) => - m(this.ERROR_UnparseableTransformFrom, `Invalid transform from="${def(o.from)}": "${def(o.message)}"`); + static Error_UnparseableTransformFrom = (o: { from: string, message: string }, x?: ObjectWithMetadata) => mx( + this.ERROR_UnparseableTransformFrom, x, + `Invalid transform from="${def(o.from)}": "${def(o.message)}"`, + ); //------------------------------------------------------------------------------| // max length of detail message lines (checked by verifyCompilerMessagesObject) | //------------------------------------------------------------------------------| static ERROR_IllegalTransformDollarsign = SevErrorTransform | 0x01; - static Error_IllegalTransformDollarsign = (o: { from: string }) => m( - this.ERROR_IllegalTransformDollarsign, + static Error_IllegalTransformDollarsign = (o: { from: string }, x?: ObjectWithMetadata) => mx( + this.ERROR_IllegalTransformDollarsign, x, `Invalid transform from="${def(o.from)}": Unescaped dollar-sign ($) is not valid transform syntax.`, ` **Hint**: Use \`\\$\` to match a literal dollar-sign. If this precedes a variable name, the variable name may not be valid (A-Z, a-z, 0-9, _, 32 @@ -377,30 +382,30 @@ export class LdmlCompilerMessages { ); static ERROR_IllegalTransformPlus = SevErrorTransform | 0x03; - static Error_IllegalTransformPlus = (o: { from: string }) => m( - this.ERROR_IllegalTransformPlus, + static Error_IllegalTransformPlus = (o: { from: string }, x?: ObjectWithMetadata) => mx( + this.ERROR_IllegalTransformPlus, x, `Invalid transform from="${def(o.from)}": Unescaped plus (+) is not valid transform syntax.`, ` **Hint**: Use \`\\+\` to match a literal plus. `); static ERROR_IllegalTransformAsterisk = SevErrorTransform | 0x04; - static Error_IllegalTransformAsterisk = (o: { from: string }) =>m( - this.ERROR_IllegalTransformAsterisk, + static Error_IllegalTransformAsterisk = (o: { from: string }, x?: ObjectWithMetadata) =>mx( + this.ERROR_IllegalTransformAsterisk, x, `Invalid transform from="${def(o.from)}": Unescaped asterisk (*) is not valid transform syntax.`, ` **Hint**: Use \`\\*\` to match a literal asterisk. `); static ERROR_IllegalTransformToUset = SevErrorTransform | 0x05; - static Error_IllegalTransformToUset = (o: { to: string }) => m( - this.ERROR_IllegalTransformToUset, + static Error_IllegalTransformToUset = (o: { to: string }, x?: ObjectWithMetadata) => mx( + this.ERROR_IllegalTransformToUset, x, `Invalid transform to="${def(o.to)}": Set variable (\\$[…]) cannot be used in 'to=' unless part of a map.`, ` **Hint**: If a map was meant, must use the form \`\`. `); static ERROR_UnparseableTransformTo = SevErrorTransform | 0x06; - static Error_UnparseableTransformTo = (o: {to: string, message: string}) => m( - this.ERROR_UnparseableTransformTo, + static Error_UnparseableTransformTo = (o: {to: string, message: string}, x?: ObjectWithMetadata) => mx( + this.ERROR_UnparseableTransformTo, x, `Invalid transform to="${def(o.to)}": "${def(o.message)}"`, ); diff --git a/developer/src/kmc-ldml/src/compiler/tran.ts b/developer/src/kmc-ldml/src/compiler/tran.ts index f0db46336d..c6236c1be7 100644 --- a/developer/src/kmc-ldml/src/compiler/tran.ts +++ b/developer/src/kmc-ldml/src/compiler/tran.ts @@ -1,6 +1,6 @@ import { constants, SectionIdent } from "@keymanapp/ldml-keyboard-constants"; import { KMXPlus, LdmlKeyboardTypes, util } from '@keymanapp/common-types'; -import { CompilerCallbacks, LDMLKeyboard } from "@keymanapp/developer-utils"; +import { CompilerCallbacks, LDMLKeyboard, ObjectWithMetadata } from "@keymanapp/developer-utils"; import { SectionCompiler } from "./section-compiler.js"; import Bksp = KMXPlus.Bksp; @@ -204,7 +204,7 @@ export abstract class TransformCompiler(); /** takes line numbers */ let lines = 0; const fakeOffsetNumber = 1234; const fakeOffsetObject = withOffset(fakeOffsetNumber); for(const key of keys) { - if(typeof m[key] == 'function') { + // exclude this one, does not need line numbers + if (key == 'Error_InvalidFile') continue; + 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++; + noLines.add(key); continue; } // now try to call it @@ -42,16 +44,11 @@ describe('LdmlCompilerMessages', function () { 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++; + noLines.add(key); } } } 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} ${Number((noLines/total)*1000).toFixed(0)}‰ of message(s) did not have detectable offset (line number) reporting.`); - } + expect(Array.from(noLines.values())).to.deep.equal([], `${noLines.size}/${total} ${Number((noLines.size/total)*100).toFixed(0)}% of message(s) did not have detectable offset (line number) reporting`); }); }); From e1638ad4589592124687ce884bfa14e3582e2082 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Sat, 31 May 2025 07:46:11 -0500 Subject: [PATCH 2/2] feat(comon): ldml: test fix for element strings with context Fixes: #13937 --- .../kmx/kmx-plus/element-string.tests.ts | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/common/web/types/tests/kmx/kmx-plus/element-string.tests.ts b/common/web/types/tests/kmx/kmx-plus/element-string.tests.ts index 49a4c2c954..3818f7022b 100644 --- a/common/web/types/tests/kmx/kmx-plus/element-string.tests.ts +++ b/common/web/types/tests/kmx/kmx-plus/element-string.tests.ts @@ -91,12 +91,12 @@ describe('Test of ElementString file', () => { }); describe('Test of fromStrings()', () => { it('returns an empty ElementString if source is null', () => { - const es = ElementString.fromStrings({}, null); + const es = ElementString.fromStrings({}, {}, null); assert.deepEqual(es, new ElementString()); }); it('can create an ElementString from a string array', () => { sections.strs.allocString = stubStrsAllocString_Char; - const actual = ElementString.fromStrings(sections, ["𐌰", "𐌱", "𐌲"]); + const actual = ElementString.fromStrings(sections, {}, ["𐌰", "𐌱", "𐌲"]); const expected = [ initElemElement(GOTHIC_A), initElemElement(GOTHIC_B), @@ -106,7 +106,7 @@ describe('Test of ElementString file', () => { }); it('can create an ElementString from a string', () => { sections.strs.allocString = stubStrsAllocString_Char; - const actual = ElementString.fromStrings(sections, "𐌰𐌱𐌲"); + const actual = ElementString.fromStrings(sections, {}, "𐌰𐌱𐌲"); const expected = [ initElemElement(GOTHIC_A), initElemElement(GOTHIC_B), @@ -118,6 +118,7 @@ describe('Test of ElementString file', () => { sections.strs.allocString = stubStrsAllocString_Char; const actual = ElementString.fromStrings( sections, + {}, "𐌰𐌱𐌲", "1 2 3", ); @@ -132,6 +133,7 @@ describe('Test of ElementString file', () => { sections.strs.allocString = stubStrsAllocString_Char; const actual = ElementString.fromStrings( sections, + {}, "𐌰𐌱𐌲", "1", ); @@ -146,6 +148,7 @@ describe('Test of ElementString file', () => { sections.strs.allocString = stubStrsAllocString_Char; const actual = ElementString.fromStrings( sections, + {}, "𐌰𐌱𐌲", "1 2", ); @@ -160,6 +163,7 @@ describe('Test of ElementString file', () => { sections.strs.allocString = stubStrsAllocString_Char; const actual = ElementString.fromStrings( sections, + {}, "𐌰𐌱𐌲", "1 A 3", ); @@ -174,6 +178,7 @@ describe('Test of ElementString file', () => { sections.strs.allocString = stubStrsAllocString_Char; const actual = ElementString.fromStrings( sections, + {}, "𐌰𐌱𐌲", null, "1 2 3", @@ -189,6 +194,7 @@ describe('Test of ElementString file', () => { sections.strs.allocString = stubStrsAllocString_Char; const actual = ElementString.fromStrings( sections, + {}, "𐌰𐌱𐌲", null, "1", @@ -204,6 +210,7 @@ describe('Test of ElementString file', () => { sections.strs.allocString = stubStrsAllocString_Char; const actual = ElementString.fromStrings( sections, + {}, "𐌰𐌱𐌲", null, "1 2", @@ -219,6 +226,7 @@ describe('Test of ElementString file', () => { sections.strs.allocString = stubStrsAllocString_Char; const actual = ElementString.fromStrings( sections, + {}, "𐌰𐌱𐌲", null, "1 A 3", @@ -234,6 +242,7 @@ describe('Test of ElementString file', () => { sections.strs.allocString = stubStrsAllocString_Char; const actual = ElementString.fromStrings( sections, + {}, "𐌰𐌱𐌲", null, null, @@ -250,6 +259,7 @@ describe('Test of ElementString file', () => { sections.strs.allocString = stubStrsAllocString_Char; const actual = ElementString.fromStrings( sections, + {}, "𐌰𐌱𐌲", null, null, @@ -266,6 +276,7 @@ describe('Test of ElementString file', () => { sections.strs.allocString = stubStrsAllocString_Char; const actual = ElementString.fromStrings( sections, + {}, "𐌰𐌱𐌲", null, null, @@ -282,6 +293,7 @@ describe('Test of ElementString file', () => { sections.strs.allocString = stubStrsAllocString_Char; const actual = ElementString.fromStrings( sections, + {}, "𐌰𐌱𐌲", null, null, @@ -299,6 +311,7 @@ describe('Test of ElementString file', () => { sections.strs.allocString = stubStrsAllocString_Char; const actual = ElementString.fromStrings( sections, + {}, "𐌰𐌱𐌲", null, null, @@ -316,6 +329,7 @@ describe('Test of ElementString file', () => { sections.strs.allocString = stubStrsAllocString_Char; const actual = ElementString.fromStrings( sections, + {}, "𐌰𐌱𐌲", null, null, @@ -333,7 +347,7 @@ describe('Test of ElementString file', () => { ElementParser.segment = stubElementParserSegment_Uset; sections.strs.allocString = stubStrsAllocString_Str; sections.uset.allocUset = stubUsetAllocUset; - const actual = ElementString.fromStrings(sections, "[𐌰-𐍊]"); + const actual = ElementString.fromStrings(sections, {}, "[𐌰-𐍊]"); const expected = [ initElemElement( new StrsItem(''), @@ -348,17 +362,17 @@ describe('Test of ElementString file', () => { it('returns null for an invalid unicode set size', () => { ElementParser.segment = stubElementParserSegment_Uset; sections.usetparser.sizeUnicodeSet = (pattern: string) : number => -1; - assert.isNull(ElementString.fromStrings(sections, "[𐌰-𐍊]")); + assert.isNull(ElementString.fromStrings(sections, {}, "[𐌰-𐍊]")); }); it('returns null if it cannot parse the unicode set', () => { ElementParser.segment = stubElementParserSegment_Uset; sections.usetparser.parseUnicodeSet = (pattern: string, rangeCount: number) : UnicodeSet | null => null; - assert.isNull(ElementString.fromStrings(sections, "[𐌰-𐍊]")); + assert.isNull(ElementString.fromStrings(sections, {}, "[𐌰-𐍊]")); }); it('can handle quad strings', () => { sections.strs.allocString = stubStrsAllocString_Char; ElementParser.segment = stubElementParserSegment_Escaped; - const actual = ElementString.fromStrings(sections, "\\ud800\\udf30"); + const actual = ElementString.fromStrings(sections, {}, "\\ud800\\udf30"); const expected = [ initElemElement(HI_GOTHIC_A), initElemElement(LO_GOTHIC_A), @@ -367,7 +381,7 @@ describe('Test of ElementString file', () => { }); it('can handle ElemElement of string type', () => { sections.strs.allocString = stubStrsAllocString_Str; - const actual = ElementString.fromStrings(sections, ["𐌰𐌱𐌲",]); + const actual = ElementString.fromStrings(sections, {}, ["𐌰𐌱𐌲",]); const expected = [ initElemElement( new StrsItem("𐌰𐌱𐌲"),