Merge pull request #14064 from keymanapp/feat/developer/13932-ln-more-strscompiler

feat(developer): update strs compiler to collect context
This commit is contained in:
Steven R. Loomis 2025-06-04 08:37:56 -05:00 committed by GitHub
commit f4476e2032
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 125 additions and 46 deletions

View file

@ -135,6 +135,18 @@ export class StrsItem {
isEqual(a: StrsItem): boolean {
return a.value === this.value && a.char === this.char;
}
private _context: any = undefined;
/** add any context from the options to this strsitem */
setContext(opts?: StrsOptions) {
// At present, there's only a single piece of context available
this._context = this._context || opts?.x;
}
get context() : any {
return this._context;
}
};
/**
@ -161,6 +173,8 @@ export interface StrsOptions {
nfd?: boolean;
/** string can be stored as a single CharStrsItem, not in strs table. */
singleOk?: boolean;
/** optional context */
x?: any;
};
export class Strs extends Section {
@ -192,6 +206,8 @@ export class Strs extends Section {
result = new StrsItem(s);
this.strings.push(result);
}
// give an option to set the context
result.setContext(opts);
return result;
}

View file

@ -61,7 +61,7 @@ export class DispCompiler extends SectionCompiler {
const result = new Disp();
// displayOptions
result.baseCharacter = sections.strs.allocString(this.keyboard3.displays?.displayOptions?.baseCharacter, {unescape: true});
result.baseCharacter = sections.strs.allocString(this.keyboard3.displays?.displayOptions?.baseCharacter, { unescape: true, x: this.keyboard3?.displays?.displayOptions });
// displays
result.disps = this.keyboard3.displays?.display.map(display => ({
@ -69,11 +69,13 @@ export class DispCompiler extends SectionCompiler {
stringVariables: true,
markers: true,
unescape: true,
x: display,
}, sections),
id: sections.strs.allocString(display.keyId), // not escaped, not substituted
id: sections.strs.allocString(display.keyId, { x: display }), // not escaped, not substituted
display: sections.strs.allocString(display.display, {
stringVariables: true,
unescape: true,
x: display,
}, sections),
})) || []; // TODO-LDML: need coverage for the []

View file

@ -1,7 +1,7 @@
import { SectionIdent, constants } from '@keymanapp/ldml-keyboard-constants';
import { SectionCompiler } from "./section-compiler.js";
import { util, KMXPlus, LdmlKeyboardTypes } from "@keymanapp/common-types";
import { CompilerCallbacks, LDMLKeyboard } from "@keymanapp/developer-utils";
import { CompilerCallbacks, LDMLKeyboard, ObjectWithMetadata } from "@keymanapp/developer-utils";
import { VarsCompiler } from './vars.js';
import { LdmlCompilerMessages } from './ldml-compiler-messages.js';
@ -33,13 +33,26 @@ export class StrsCompiler extends EmptyCompiler {
public postValidate(section?: KMXPlus.Section): boolean {
const strs = <KMXPlus.Strs>section;
/** attempt to find a context object for the string */
function findContextForString(s: string): ObjectWithMetadata {
// try exact match
for(const str of strs.strings) {
if (str.value == s) return str.context;
}
// try substring match
for(const str of strs.strings) {
if (str.value.includes(s)) return str.context;
}
return null;
}
if (strs) {
const badStringAnalyzer = new util.BadStringAnalyzer();
const CONTAINS_MARKER_REGEX = new RegExp(LdmlKeyboardTypes.MarkerParser.ANY_MARKER_MATCH);
for (let s of strs.allProcessedStrings.values()) {
// stop at the first denormalized string
if (!util.isNormalized(s)) {
this.callbacks.reportMessage(LdmlCompilerMessages.Warn_StringDenorm({s}));
this.callbacks.reportMessage(LdmlCompilerMessages.Warn_StringDenorm({s}, findContextForString(s)));
}
// replace all \\uXXXX with the actual code point.
// this lets us analyze whether there are PUA, unassigned, etc.
@ -63,16 +76,19 @@ export class StrsCompiler extends EmptyCompiler {
const illegals = m.get(util.BadStringType.illegal);
if (puas) {
const [count, lowestCh] = [puas.size, Array.from(puas.values()).sort((a, b) => a - b)[0]];
this.callbacks.reportMessage(LdmlCompilerMessages.Hint_PUACharacters({ count, lowestCh }))
this.callbacks.reportMessage(LdmlCompilerMessages.Hint_PUACharacters({ count, lowestCh },
findContextForString(String.fromCodePoint(lowestCh))));
}
if (unassigneds) {
const [count, lowestCh] = [unassigneds.size, Array.from(unassigneds.values()).sort((a, b) => a - b)[0]];
this.callbacks.reportMessage(LdmlCompilerMessages.Warn_UnassignedCharacters({ count, lowestCh }))
this.callbacks.reportMessage(LdmlCompilerMessages.Warn_UnassignedCharacters({ count, lowestCh },
findContextForString(String.fromCodePoint(lowestCh))));
}
if (illegals) {
// do this last, because we will return false.
const [count, lowestCh] = [illegals.size, Array.from(illegals.values()).sort((a, b) => a - b)[0]];
this.callbacks.reportMessage(LdmlCompilerMessages.Error_IllegalCharacters({ count, lowestCh }))
this.callbacks.reportMessage(LdmlCompilerMessages.Error_IllegalCharacters({ count, lowestCh },
findContextForString(String.fromCodePoint(lowestCh))));
return false;
}
}

View file

@ -324,12 +324,13 @@ export class KeysCompiler extends SectionCompiler {
// allocate the in-memory <flick id=…>
const flicks: KeysFlicks = new KeysFlicks(
sections.strs.allocString(flickId)
sections.strs.allocString(flickId, { x: flick })
);
// add data from each segment
for (const { keyId, directions } of flick.flickSegment) {
const keyIdStr = sections.strs.allocString(keyId);
for (const flickSegment of flick.flickSegment) {
const { keyId, directions } = flickSegment;
const keyIdStr = sections.strs.allocString(keyId, { x: flickSegment });
const directionsList: ListItem = sections.list.allocListFromSpaces(
directions,
{ },
@ -369,20 +370,20 @@ export class KeysCompiler extends SectionCompiler {
if (!!gap) {
flags |= constants.keys_key_flags_gap;
}
const id = sections.strs.allocString(key.id);
const id = sections.strs.allocString(key.id, { x: key });
const longPress: ListItem = sections.list.allocListFromSpaces(
longPressKeyIds, {},
sections);
const longPressDefault = sections.strs.allocString(longPressDefaultKeyId,
{},
{ x: key },
sections);
const multiTap: ListItem = sections.list.allocListFromSpaces(
multiTapKeyIds,
{},
sections);
const keySwitch = sections.strs.allocString(layerId); // 'switch' is a reserved word
const keySwitch = sections.strs.allocString(layerId, { x: key }); // 'switch' is a reserved word
const toRaw = output;
@ -395,6 +396,7 @@ export class KeysCompiler extends SectionCompiler {
unescape: true,
singleOk: true,
nfd: true,
x: key,
},
sections);
if (!to.isOneChar) {

View file

@ -69,13 +69,13 @@ export class LayrCompiler extends SectionCompiler {
const sect = new Layr();
sect.lists = this.keyboard3.layers.map((layers) => {
const hardware = sections.strs.allocString(layers.formId);
const hardware = sections.strs.allocString(layers.formId, {x:layers});
// Already validated in validate
const layerEntries = [];
for (const layer of layers.layer) {
const rows = layer.row.map((row) => {
const erow: LayrRow = {
keys: row.keys.trim().split(/[ \t]+/).map((id) => sections.strs.allocString(id)),
keys: row.keys.trim().split(/[ \t]+/).map((id) => sections.strs.allocString(id, { x: row })),
};
// include linenumber info for row
return SectionCompiler.copySymbols(erow, row);
@ -84,7 +84,7 @@ export class LayrCompiler extends SectionCompiler {
// push a layer entry for each modifier set
for (const mod of mods) {
layerEntries.push({
id: sections.strs.allocString(layer.id),
id: sections.strs.allocString(layer.id, {x:layer}),
mod,
rows,
});

View file

@ -267,16 +267,22 @@ export class LdmlCompilerMessages {
);
static HINT_PUACharacters = SevHint | 0x0023;
static Hint_PUACharacters = (o: { count: number, lowestCh: number }) =>
m(this.HINT_PUACharacters, `File contains ${def(o.count)} PUA character(s), including ${util.describeCodepoint(o.lowestCh)}`);
static Hint_PUACharacters = (o: { count: number, lowestCh: number }, x?: ObjectWithMetadata) => mx(
this.HINT_PUACharacters, x,
`File contains ${def(o.count)} PUA character(s), including ${util.describeCodepoint(o.lowestCh)}`,
);
static WARN_UnassignedCharacters = SevWarn | 0x0024;
static Warn_UnassignedCharacters = (o: { count: number, lowestCh: number }) =>
m(this.WARN_UnassignedCharacters, `File contains ${def(o.count)} unassigned character(s), including ${util.describeCodepoint(o.lowestCh)}`);
static Warn_UnassignedCharacters = (o: { count: number, lowestCh: number }, x?: ObjectWithMetadata) => mx(
this.WARN_UnassignedCharacters, x,
`File contains ${def(o.count)} unassigned character(s), including ${util.describeCodepoint(o.lowestCh)}`,
);
static ERROR_IllegalCharacters = SevError | 0x0025;
static Error_IllegalCharacters = (o: { count: number, lowestCh: number }) =>
m(this.ERROR_IllegalCharacters, `File contains ${def(o.count)} illegal character(s), including ${util.describeCodepoint(o.lowestCh)}`);
static Error_IllegalCharacters = (o: { count: number, lowestCh: number }, x?: ObjectWithMetadata) => mx(
this.ERROR_IllegalCharacters, x,
`File contains ${def(o.count)} illegal character(s), including ${ util.describeCodepoint(o.lowestCh) }`,
);
static HINT_CharClassImplicitDenorm = SevHint | 0x0026;
static Hint_CharClassImplicitDenorm = (o: { lowestCh: number }, x?: ObjectWithMetadata) => mx(
@ -307,8 +313,10 @@ export class LdmlCompilerMessages {
);
static WARN_StringDenorm = SevWarn | 0x002B;
static Warn_StringDenorm = (o: { s: string }) =>
m(this.WARN_StringDenorm, `File contains string "${def(o.s)}" that is neither NFC nor NFD.`);
static Warn_StringDenorm = (o: { s: string }, x?: ObjectWithMetadata) => mx(
this.WARN_StringDenorm, x,
`File contains string "${def(o.s)}" that is neither NFC nor NFD.`,
);
static ERROR_DuplicateLayerWidth = SevError | 0x002C;
static Error_DuplicateLayerWidth = (o: { minDeviceWidth: number }, x?: ObjectWithMetadata) => mx(

View file

@ -71,7 +71,7 @@ export class LocaCompiler extends SectionCompiler {
// yet include `getCanonicalLocales` but node 16 does include it so we can
// safely use it. Also well supported in modern browsers.
const canonicalLocales = (Intl as any).getCanonicalLocales(locales) as string[];
result.locales = canonicalLocales.map(locale => sections.strs.allocString(locale));
result.locales = canonicalLocales.map(locale => sections.strs.allocString(locale, {x: this.contextForLocale(locale)}));
if(result.locales.length < locales.length) {
this.callbacks.reportMessage(LdmlCompilerMessages.Hint_OneOrMoreRepeatedLocales(this.keyboard3?.locales));

View file

@ -53,12 +53,18 @@ export class MetaCompiler extends SectionCompiler {
public compile(sections: DependencySections): Meta {
const result = new Meta();
result.author = sections.strs.allocString(this.keyboard3.info?.author);
result.conform = sections.strs.allocString(this.keyboard3.conformsTo);
result.layout = sections.strs.allocString(this.keyboard3.info?.layout);
result.name = sections.strs.allocString(this.keyboard3.info?.name);
result.indicator = sections.strs.allocString(this.keyboard3.info?.indicator);
result.version = sections.strs.allocString(this.keyboard3.version?.number ?? "0.0.0");
result.author = sections.strs.allocString(this.keyboard3.info?.author,
{x: this.keyboard3.info});
result.conform = sections.strs.allocString(this.keyboard3.conformsTo,
{x: this.keyboard3});
result.layout = sections.strs.allocString(this.keyboard3.info?.layout,
{x: this.keyboard3.info});
result.name = sections.strs.allocString(this.keyboard3.info?.name,
{x: this.keyboard3.info});
result.indicator = sections.strs.allocString(this.keyboard3.info?.indicator,
{x: this.keyboard3.info});
result.version = sections.strs.allocString(this.keyboard3.version?.number ?? "0.0.0",
{x: this.keyboard3.version});
result.settings =
(this.keyboard3.settings?.normalization == "disabled" ? KeyboardSettings.normalizationDisabled : 0);
return result;

View file

@ -18,6 +18,7 @@ import { verifyValidAndUnique } from "../util/util.js";
import { LdmlCompilerMessages } from "./ldml-compiler-messages.js";
import { Substitutions, SubstitutionUse } from "./substitution-tracker.js";
import { transform_from_parse, transform_to_parse } from "../util/abnf/abnf.js";
import { StrsOptions } from "../../../../../common/web/types/src/kmx/kmx-plus/kmx-plus.js";
type TransformCompilerType = 'simple' | 'backspace';
@ -138,6 +139,8 @@ export abstract class TransformCompiler<T extends TransformCompilerType, TranBas
}
private compileTransform(sections: DependencySections, transform: LKTransform) : TranTransform {
// we have lots of strings to allocate, that will all have these options
const stropts : StrsOptions = { x: transform };
const result = new TranTransform();
// setup for serializing
result._from = transform.from;
@ -157,11 +160,11 @@ export abstract class TransformCompiler<T extends TransformCompilerType, TranBas
const mapFrom = LdmlKeyboardTypes.VariableParser.CAPTURE_SET_REFERENCE.exec(cookedFrom);
const mapTo = LdmlKeyboardTypes.VariableParser.MAPPED_SET_REFERENCE.exec(transform.to || '');
if (mapFrom && mapTo) { // TODO-LDML: error cases
result.mapFrom = sections.strs.allocString(mapFrom[1]); // var name
result.mapTo = sections.strs.allocString(mapTo[1]); // var name
result.mapFrom = sections.strs.allocString(mapFrom[1], stropts); // var name
result.mapTo = sections.strs.allocString(mapTo[1], stropts); // var name
} else {
result.mapFrom = sections.strs.allocString('');
result.mapTo = sections.strs.allocString('');
result.mapFrom = sections.strs.allocString('', stropts);
result.mapTo = sections.strs.allocString('', stropts);
// validate 'to' here
if (!this.isValidTo(transform.to || '')) {
@ -214,6 +217,7 @@ export abstract class TransformCompiler<T extends TransformCompilerType, TranBas
// cookedFrom is cooked above, since there's some special treatment
result.from = sections.strs.allocString(cookedFrom, {
unescape: false,
x: transform,
}, sections);
// 'to' is handled via allocString
result.to = sections.strs.allocString(transform.to, {
@ -221,6 +225,7 @@ export abstract class TransformCompiler<T extends TransformCompilerType, TranBas
markers: true,
unescape: true,
nfd: true,
x: transform,
}, sections);
return result;
}

View file

@ -1,7 +1,7 @@
import 'mocha';
import {assert} from 'chai';
import hextobin from '@keymanapp/hextobin';
import {compileKeyboard, compilerTestCallbacks, compilerTestOptions, makePathToFixture} from './helpers/index.js';
import {compileKeyboard, compilerTestCallbacks, compilerTestOptions, makePathToFixture, scrubContextFromMessages} from './helpers/index.js';
import { compareXml } from './helpers/compareXml.js';
import { LdmlKeyboardCompiler } from '../src/compiler/compiler.js';
import { kmxToXml } from '../src/util/serialize.js';
@ -53,7 +53,7 @@ describe('compiler-tests', function() {
const runOutput = await k.run(inputFilename, "invalid-illegal.kmx"); // need the exact name passed to build-fixtures
assert.isNull(runOutput, "Expect invalid-illegal to fail to run()");
assert.sameDeepMembers(compilerTestCallbacks.messages, [
assert.sameDeepMembers( scrubContextFromMessages(compilerTestCallbacks.messages), [
// copied from strs.tests.ts
// validation messages
LdmlCompilerMessages.Error_IllegalCharacters({ count: 5, lowestCh: 0xFDD0 }),

View file

@ -143,6 +143,7 @@ export async function compileKeyboard(inputFilename: string, options: LdmlCompil
assert.isNotNull(source, 'k.load should not have returned null');
const valid = await k.validate(source);
zapMessageMetadata();
if (validateMessages) {
assert.sameDeepMembers(compilerTestCallbacks.messages, validateMessages, "validation messages mismatch");
assert.notEqual(valid, expectFailValidate, 'validation failure');
@ -154,6 +155,7 @@ export async function compileKeyboard(inputFilename: string, options: LdmlCompil
if (!valid) return null; // get out, if the above asserts didn't get us out.
const kmx = await k.compile(source);
zapMessageMetadata();
if (compileMessages) {
assert.sameDeepMembers(compilerTestCallbacks.messages, compileMessages, "compiler messages mismatch");
} else {
@ -172,6 +174,16 @@ export function checkMessages() {
assert.isEmpty(compilerTestCallbacks.messages, compilerEventFormat(compilerTestCallbacks.messages));
}
/** These tests aren't prepared for line number information in messages. Remove it so that comparisons pass. */
function zapMessageMetadata() {
for(const i in compilerTestCallbacks.messages) {
delete compilerTestCallbacks.messages[i].column;
delete compilerTestCallbacks.messages[i].filename;
delete compilerTestCallbacks.messages[i].line;
delete compilerTestCallbacks.messages[i].offset;
}
}
/**
* Like CompilerEvent, but supports regex matching.
*/
@ -259,6 +271,26 @@ export interface CompilationCase {
retainOffsetInMessages?: boolean;
}
/**
* Scrub 'context' from messages. to simplify unit tests
* @param messages input array of messages
* @returns copy of messages
*/
export function scrubContextFromMessages(messages: CompilerEvent[]): CompilerEvent[] {
return 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;
delete scrubbed.line;
delete scrubbed.filename;
delete scrubbed.column;
return scrubbed;
});
}
/**
* Run a bunch of cases
* @param cases cases to run
@ -283,15 +315,7 @@ export function testCompilationCases(compiler: SectionCompilerNew, cases : Compi
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;
});
messagesToCheck = scrubContextFromMessages(callbacks.messages);
}
const testcaseErrors = matchCompilerEventsOrBoolean(messagesToCheck, testcase.errors);
const testcaseWarnings = matchCompilerEvents(messagesToCheck, testcase.warnings);