From bb63ba8a664309d126cf128cd0feb12e17073a66 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Sat, 25 Nov 2023 13:51:53 -0600 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Marc Durdin --- common/web/types/src/util/util.ts | 2 +- developer/src/kmc-ldml/src/compiler/messages.ts | 6 +++--- developer/src/kmc-ldml/test/test-compiler-e2e.ts | 13 ++++--------- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/common/web/types/src/util/util.ts b/common/web/types/src/util/util.ts index 863509ebb6..87ba3236c7 100644 --- a/common/web/types/src/util/util.ts +++ b/common/web/types/src/util/util.ts @@ -167,7 +167,7 @@ function Uni_IsNoncharacter(ch : number) { } function Uni_InCodespace(ch : number) { - return ((ch) <= Uni_MAX_CODEPOINT); + return (ch >= 0 && ch <= Uni_MAX_CODEPOINT); }; function Uni_IsValid1(ch: number) { diff --git a/developer/src/kmc-ldml/src/compiler/messages.ts b/developer/src/kmc-ldml/src/compiler/messages.ts index d26073fd35..750cc38c0f 100644 --- a/developer/src/kmc-ldml/src/compiler/messages.ts +++ b/developer/src/kmc-ldml/src/compiler/messages.ts @@ -147,15 +147,15 @@ export class CompilerMessages { static ERROR_DisplayNeedsToOrId = SevError | 0x0022; static Hint_PUACharacters = (o: { count: number, lowestCh: number }) => - m(this.HINT_PUACharacters, `File contained ${o.count} PUA character(s), including ${util.describeCodepoint(o.lowestCh)}`); + m(this.HINT_PUACharacters, `File contains ${o.count} PUA character(s), including ${util.describeCodepoint(o.lowestCh)}`); static HINT_PUACharacters = SevHint | 0x0023; static Warn_UnassignedCharacters = (o: { count: number, lowestCh: number }) => - m(this.WARN_UnassignedCharacters, `File contained ${o.count} unassigned character(s), including ${util.describeCodepoint(o.lowestCh)}`); + m(this.WARN_UnassignedCharacters, `File contains ${o.count} unassigned character(s), including ${util.describeCodepoint(o.lowestCh)}`); static WARN_UnassignedCharacters = SevWarn | 0x0024; static Error_IllegalCharacters = (o: { count: number, lowestCh: number }) => - m(this.ERROR_IllegalCharacters, `File contained ${o.count} illegal character(s), including ${util.describeCodepoint(o.lowestCh)}`); + m(this.ERROR_IllegalCharacters, `File contains ${o.count} illegal character(s), including ${util.describeCodepoint(o.lowestCh)}`); static ERROR_IllegalCharacters = SevError | 0x0025; } diff --git a/developer/src/kmc-ldml/test/test-compiler-e2e.ts b/developer/src/kmc-ldml/test/test-compiler-e2e.ts index f13c2594bd..c58d623f33 100644 --- a/developer/src/kmc-ldml/test/test-compiler-e2e.ts +++ b/developer/src/kmc-ldml/test/test-compiler-e2e.ts @@ -9,8 +9,11 @@ import { CompilerMessages } from '../src/compiler/messages.js'; describe('compiler-tests', function() { this.slow(500); // 0.5 sec -- json schema validation takes a while + before(function() { + compilerTestCallbacks.clear(); + }); + it('should-build-fixtures', async function() { - compilerTestCallbacks.messages = []; // Let's build basic.xml // It should match basic.kmx (built from basic.txt) @@ -34,42 +37,36 @@ describe('compiler-tests', function() { }); it('should handle non existent files', () => { - compilerTestCallbacks.messages = []; const filename = 'DOES_NOT_EXIST.xml'; const k = new LdmlKeyboardCompiler(compilerTestCallbacks, { ...compilerTestOptions, saveDebug: true, shouldAddCompilerVersion: false }); const source = k.load(filename); assert.notOk(source, `Trying to load(${filename})`); }); it('should handle unparseable files', () => { - compilerTestCallbacks.messages = []; const filename = makePathToFixture('basic-kvk.txt'); // not an .xml file const k = new LdmlKeyboardCompiler(compilerTestCallbacks, { ...compilerTestOptions, saveDebug: true, shouldAddCompilerVersion: false }); const source = k.load(filename); assert.notOk(source, `Trying to load(${filename})`); }); it('should handle not-valid files', () => { - compilerTestCallbacks.messages = []; const filename = makePathToFixture('test-fr.xml'); // not a keyboard .xml file const k = new LdmlKeyboardCompiler(compilerTestCallbacks, { ...compilerTestOptions, saveDebug: true, shouldAddCompilerVersion: false }); const source = k.load(filename); assert.notOk(source, `Trying to load(${filename})`); }); it('should handle non existent test files', () => { - compilerTestCallbacks.messages = []; const filename = 'DOES_NOT_EXIST.xml'; const k = new LdmlKeyboardCompiler(compilerTestCallbacks, { ...compilerTestOptions, saveDebug: true, shouldAddCompilerVersion: false }); const source = k.loadTestData(filename); assert.notOk(source, `Trying to loadTestData(${filename})`); }); it('should handle unparseable test files', () => { - compilerTestCallbacks.messages = []; const filename = makePathToFixture('basic-kvk.txt'); // not an .xml file const k = new LdmlKeyboardCompiler(compilerTestCallbacks, { ...compilerTestOptions, saveDebug: true, shouldAddCompilerVersion: false }); const source = k.load(filename); assert.notOk(source, `Trying to loadTestData(${filename})`); }); it('should fail on illegal chars', async function() { - compilerTestCallbacks.messages = []; const inputFilename = makePathToFixture('sections/strs/invalid-illegal.xml'); const kmx = await compileKeyboard(inputFilename, { ...compilerTestOptions, saveDebug: true, shouldAddCompilerVersion: false }, [ @@ -84,7 +81,6 @@ describe('compiler-tests', function() { assert.isNull(kmx); // should fail post-validate }); it('should hint on pua chars', async function() { - compilerTestCallbacks.messages = []; const inputFilename = makePathToFixture('sections/strs/hint-pua.xml'); // Compile the keyboard const kmx = await compileKeyboard(inputFilename, { ...compilerTestOptions, saveDebug: true, shouldAddCompilerVersion: false }, @@ -101,7 +97,6 @@ describe('compiler-tests', function() { }); it.skip('should warn on unassigned chars', async function() { // unassigned not implemented yet - compilerTestCallbacks.messages = []; const inputFilename = makePathToFixture('sections/strs/warn-unassigned.xml'); const kmx = await compileKeyboard(inputFilename, { ...compilerTestOptions, saveDebug: true, shouldAddCompilerVersion: false }, [