feat(developer): marker accounting 🙀

- split out MarkerTracker, could give us more precise messages about marker use
- for now, we parse all markers twice.
- update builder for the markers list

#9119
This commit is contained in:
Steven R. Loomis 2023-07-29 14:18:37 -05:00
parent 4854cba4c9
commit 52e54395f9
10 changed files with 217 additions and 80 deletions

View file

@ -2,7 +2,7 @@ import { constants } from "@keymanapp/ldml-keyboard-constants";
import { KMXPlusData } from "../kmx-plus.js";
import { build_strs_index, BUILDER_STR_REF, BUILDER_STRS } from "./build-strs.js";
import { BUILDER_SECTION } from "./builder-section.js";
import { BUILDER_LIST_REF } from "./build-list.js";
import { build_list_index, BUILDER_LIST, BUILDER_LIST_REF } from "./build-list.js";
import { build_elem_index, BUILDER_ELEM, BUILDER_ELEM_REF } from "./build-elem.js";
@ -22,7 +22,7 @@ export interface BUILDER_VARS extends BUILDER_SECTION {
/**
* Builder for the 'vars' section
*/
export function build_vars(kmxplus: KMXPlusData, sect_strs: BUILDER_STRS, sect_elem: BUILDER_ELEM) : BUILDER_VARS {
export function build_vars(kmxplus: KMXPlusData, sect_strs: BUILDER_STRS, sect_elem: BUILDER_ELEM, sect_list: BUILDER_LIST) : BUILDER_VARS {
if(!kmxplus.vars) {
return null;
}
@ -49,7 +49,7 @@ export function build_vars(kmxplus: KMXPlusData, sect_strs: BUILDER_STRS, sect_e
size: constants.length_vars +
(constants.length_vars_item * kmxplus.vars.totalCount()),
_offset: 0,
markers: 0,
markers: build_list_index(sect_list, kmxplus.vars.markers),
varCount: kmxplus.vars.totalCount(),
varEntries: [
...stringVars,

View file

@ -99,7 +99,7 @@ export default class KMXPlusBuilder {
this.sect.name = build_name(this.file.kmxplus, this.sect.strs);
this.sect.tran = build_tran(this.file.kmxplus.tran, this.sect.strs, this.sect.elem);
this.sect.uset = build_uset(this.file.kmxplus, this.sect.strs);
this.sect.vars = build_vars(this.file.kmxplus, this.sect.strs, this.sect.elem);
this.sect.vars = build_vars(this.file.kmxplus, this.sect.strs, this.sect.elem, this.sect.list);
this.sect.vkey = build_vkey(this.file.kmxplus);
// Finalize the sect (index) section

View file

@ -68,7 +68,12 @@ export class ListItem extends Array<ListIndex> {
return 0;
}
}
/** for debugging, print as single string */
toString(): string {
return this.map(v => v.value.value).join(' ');
return this.toStringArray().join(' ');
}
/** for debugging, map to string array */
toStringArray(): string[] {
return this.map(v => v.value.value);
}
};

View file

@ -7,12 +7,12 @@ import { SectionCompiler } from "./section-compiler.js";
import DependencySections = KMXPlus.DependencySections;
import Disp = KMXPlus.Disp;
import DispItem = KMXPlus.DispItem;
import { MarkerTracker, MarkerUse } from "./marker-tracker.js";
export class DispCompiler extends SectionCompiler {
static validateMarkers(keyboard: LDMLKeyboard.LKKeyboard, emitMarkers: Set<string>, matchMarkers: Set<string>): boolean {
keyboard.displays?.display?.forEach(({ to }) => {
MarkerParser.allReferences(to).forEach(marker => matchMarkers.add(marker));
});
static validateMarkers(keyboard: LDMLKeyboard.LKKeyboard, mt : MarkerTracker): boolean {
keyboard.displays?.display?.forEach(({ to }) =>
mt.add(MarkerUse.match, MarkerParser.allReferences(to)));
return true;
}

View file

@ -8,12 +8,16 @@ import Keys = KMXPlus.Keys;
import ListItem = KMXPlus.ListItem;
import KeysFlicks = KMXPlus.KeysFlicks;
import { allUsedKeyIdsInLayers, calculateUniqueKeys, translateLayerAttrToModifier, validModifier } from '../util/util.js';
import { MarkerTracker, MarkerUse } from './marker-tracker.js';
export class KeysCompiler extends SectionCompiler {
static validateMarkers(keyboard: LDMLKeyboard.LKKeyboard, emitMarkers: Set<string>, matchMarkers: Set<string>): boolean {
keyboard.keys?.key?.forEach(({ to }) => {
MarkerParser.allReferences(to).forEach(marker => emitMarkers.add(marker));
});
static validateMarkers(
keyboard: LDMLKeyboard.LKKeyboard,
mt: MarkerTracker
): boolean {
keyboard.keys?.key?.forEach(({ to }) =>
mt.add(MarkerUse.emit, MarkerParser.allReferences(to))
);
return true;
}
@ -26,7 +30,7 @@ export class KeysCompiler extends SectionCompiler {
* @returns just the non-touch layers.
*/
public hardwareLayers() {
return this.keyboard.layers?.filter(({form}) => form !== 'touch');
return this.keyboard.layers?.filter(({ form }) => form !== "touch");
}
public validate() {
@ -36,7 +40,7 @@ export class KeysCompiler extends SectionCompiler {
const usedKeys = allUsedKeyIdsInLayers(this.keyboard?.layers);
const uniqueKeys = calculateUniqueKeys([...this.keyboard.keys?.key]);
for (let key of uniqueKeys) {
const {id, flicks} = key;
const { id, flicks } = key;
if (!usedKeys.has(id)) {
continue; // unused key, ignore
}
@ -44,10 +48,14 @@ export class KeysCompiler extends SectionCompiler {
if (!flicks) {
continue; // no flicks
}
const flickEntry = this.keyboard.keys?.flicks?.find(x => x.id === flicks);
if (!flickEntry ) {
const flickEntry = this.keyboard.keys?.flicks?.find(
(x) => x.id === flicks
);
if (!flickEntry) {
valid = false;
this.callbacks.reportMessage(CompilerMessages.Error_MissingFlicks({flicks, id}));
this.callbacks.reportMessage(
CompilerMessages.Error_MissingFlicks({ flicks, id })
);
}
}
@ -59,8 +67,9 @@ export class KeysCompiler extends SectionCompiler {
if (hardwareLayers.length >= 1) {
// validate all errors
for (let layers of hardwareLayers) {
for(let layer of layers.layer) {
valid = this.validateHardwareLayerForKmap(layers.form, layer) && valid; // note: always validate even if previously invalid results found
for (let layer of layers.layer) {
valid =
this.validateHardwareLayerForKmap(layers.form, layer) && valid; // note: always validate even if previously invalid results found
}
}
// TODO-LDML: } else { touch?
@ -90,11 +99,13 @@ export class KeysCompiler extends SectionCompiler {
/* c8 ignore next 3 */
if (hardwareLayers.length > 1) {
// validation should have already caught this
throw Error(`Internal error: Expected 0 or 1 hardware layer, not ${hardwareLayers.length}`);
throw Error(
`Internal error: Expected 0 or 1 hardware layer, not ${hardwareLayers.length}`
);
} else if (hardwareLayers.length === 1) {
const theLayers = hardwareLayers[0];
const { form } = theLayers;
for(let layer of theLayers.layer) {
for (let layer of theLayers.layer) {
this.compileHardwareLayerToKmap(sections, layer, sect, form);
}
} // else: TODO-LDML do nothing if only touch layers
@ -104,7 +115,9 @@ export class KeysCompiler extends SectionCompiler {
public loadFlicks(sections: DependencySections, sect: Keys) {
for (let lkflicks of this.keyboard.keys.flicks) {
let flicks: KeysFlicks = new KeysFlicks(sections.strs.allocString(lkflicks.id));
let flicks: KeysFlicks = new KeysFlicks(
sections.strs.allocString(lkflicks.id)
);
for (let lkflick of lkflicks.flick) {
let flags = 0;
@ -112,7 +125,10 @@ export class KeysCompiler extends SectionCompiler {
if (!to.isOneChar) {
flags |= constants.keys_flick_flags_extend;
}
let directions: ListItem = sections.list.allocListFromSpaces(sections.strs, lkflick.directions);
let directions: ListItem = sections.list.allocListFromSpaces(
sections.strs,
lkflick.directions
);
flicks.flicks.push({
directions,
flags,
@ -138,19 +154,27 @@ export class KeysCompiler extends SectionCompiler {
if (!!key.gap) {
flags |= constants.keys_key_flags_gap;
}
if (key.transform === 'no') {
if (key.transform === "no") {
flags |= constants.keys_key_flags_notransform;
}
const id = sections.strs.allocString(key.id);
const longPress: ListItem = sections.list.allocListFromEscapedSpaces(sections.strs, key.longPress);
const longPressDefault = sections.strs.allocAndUnescapeString(key.longPressDefault);
const multiTap: ListItem = sections.list.allocListFromEscapedSpaces(sections.strs, key.multiTap);
const longPress: ListItem = sections.list.allocListFromEscapedSpaces(
sections.strs,
key.longPress
);
const longPressDefault = sections.strs.allocAndUnescapeString(
key.longPressDefault
);
const multiTap: ListItem = sections.list.allocListFromEscapedSpaces(
sections.strs,
key.multiTap
);
const keySwitch = sections.strs.allocString(key.switch); // 'switch' is a reserved word
const to = sections.strs.allocAndUnescapeString(key.to, true);
if (!to.isOneChar) {
flags |= constants.keys_key_flags_extend;
}
const width = Math.ceil((key.width || 1) * 10.0); // default, width=1
const width = Math.ceil((key.width || 1) * 10.0); // default, width=1
sect.keys.push({
flags,
flicks,
@ -172,12 +196,17 @@ export class KeysCompiler extends SectionCompiler {
* @param layer
* @returns
*/
private validateHardwareLayerForKmap(hardware: string, layer: LDMLKeyboard.LKLayer) {
private validateHardwareLayerForKmap(
hardware: string,
layer: LDMLKeyboard.LKLayer
) {
let valid = true;
const { modifier } = layer;
if (!validModifier(modifier)) {
this.callbacks.reportMessage(CompilerMessages.Error_InvalidModifier({ modifier, layer: layer.id }));
this.callbacks.reportMessage(
CompilerMessages.Error_InvalidModifier({ modifier, layer: layer.id })
);
valid = false;
}
@ -185,21 +214,31 @@ export class KeysCompiler extends SectionCompiler {
/* c8 ignore next 5 */
if (!keymap) {
// not reached due to XML validation
this.callbacks.reportMessage(CompilerMessages.Error_InvalidHardware({ form: hardware }));
this.callbacks.reportMessage(
CompilerMessages.Error_InvalidHardware({ form: hardware })
);
valid = false;
}
const uniqueKeys = calculateUniqueKeys([...this.keyboard.keys?.key]);
if (layer.row.length > keymap.length) {
this.callbacks.reportMessage(CompilerMessages.Error_HardwareLayerHasTooManyRows());
this.callbacks.reportMessage(
CompilerMessages.Error_HardwareLayerHasTooManyRows()
);
valid = false;
}
for (let y = 0; y < layer.row.length && y < keymap.length; y++) {
const keys = layer.row[y].keys.split(' ');
const keys = layer.row[y].keys.split(" ");
if (keys.length > keymap[y].length) {
this.callbacks.reportMessage(CompilerMessages.Error_RowOnHardwareLayerHasTooManyKeys({ row: y + 1, hardware, modifier }));
this.callbacks.reportMessage(
CompilerMessages.Error_RowOnHardwareLayerHasTooManyKeys({
row: y + 1,
hardware,
modifier,
})
);
valid = false;
}
@ -207,14 +246,24 @@ export class KeysCompiler extends SectionCompiler {
for (let key of keys) {
x++;
let keydef = uniqueKeys.find(x => x.id == key);
let keydef = uniqueKeys.find((x) => x.id == key);
if (!keydef) {
this.callbacks.reportMessage(CompilerMessages.Error_KeyNotFoundInKeyBag({ keyId: key, col: x + 1, row: y + 1, layer: layer.id, form: 'hardware' }));
this.callbacks.reportMessage(
CompilerMessages.Error_KeyNotFoundInKeyBag({
keyId: key,
col: x + 1,
row: y + 1,
layer: layer.id,
form: "hardware",
})
);
valid = false;
continue;
}
if (!keydef.to && !keydef.gap && !keydef.switch) {
this.callbacks.reportMessage(CompilerMessages.Error_KeyMissingToGapOrSwitch({ keyId: key }));
this.callbacks.reportMessage(
CompilerMessages.Error_KeyMissingToGapOrSwitch({ keyId: key })
);
valid = false;
continue;
}
@ -228,7 +277,7 @@ export class KeysCompiler extends SectionCompiler {
sections: DependencySections,
layer: LDMLKeyboard.LKLayer,
sect: Keys,
hardware: string,
hardware: string
): Keys {
const mod = translateLayerAttrToModifier(layer);
const keymap = Constants.HardwareToKeymap.get(hardware);
@ -237,7 +286,7 @@ export class KeysCompiler extends SectionCompiler {
for (let row of layer.row) {
y++;
const keys = row.keys.split(' ');
const keys = row.keys.split(" ");
let x = -1;
for (let key of keys) {
x++;

View file

@ -0,0 +1,72 @@
/**
* Verb for MarkerTracker.add()
*/
export enum MarkerUse {
/** outputs this marker into context (e.g. transform to= or key to=) */
emit,
/** consumes this marker out of the context (e.g. transform from=) */
consume,
/** matches the marker, but doesn't consume (e.g. display to=) */
match,
/** variable definition: might consume, emit, or match. */
variable,
}
type MarkerSet = Set<string>;
/** Tracks usage of markers */
export class MarkerTracker {
/** markers that were emitted */
emitted: MarkerSet;
/** markers that were consumed and removed from the context */
consumed: MarkerSet;
/** markers that were matched, but not necessarily consumed */
matched: MarkerSet;
/** all markers */
all: MarkerSet;
constructor() {
this.emitted = new Set<string>();
this.consumed = new Set<string>();
this.matched = new Set<string>();
this.all = new Set<string>();
}
/**
*
* @param verb what kind of use we are adding
* @param markers list of markers to add
*/
add(verb: MarkerUse, markers: string[]) {
if (!markers.length) {
return; // skip if empty
}
if (verb == MarkerUse.emit) {
markers.forEach((m) => {
this.emitted.add(m);
this.all.add(m);
});
} else if (verb == MarkerUse.consume) {
markers.forEach((m) => {
this.consumed.add(m);
this.all.add(m);
});
} else if (verb == MarkerUse.match) {
markers.forEach((m) => {
this.matched.add(m);
this.all.add(m);
});
} else if (verb == MarkerUse.variable) {
markers.forEach((m) => {
// we don't know, so add it to all three
this.matched.add(m);
this.emitted.add(m);
this.consumed.add(m);
this.all.add(m);
});
/* c8 skip next 3 */
} else {
throw Error(`Internal error: unsupported verb ${verb} for match`);
}
}
}

View file

@ -15,19 +15,19 @@ import LKTransform = LDMLKeyboard.LKTransform;
import LKTransforms = LDMLKeyboard.LKTransforms;
import { verifyValidAndUnique } from "../util/util.js";
import { CompilerMessages } from "./messages.js";
import { MarkerTracker, MarkerUse } from "./marker-tracker.js";
type TransformCompilerType = 'simple' | 'backspace';
export class TransformCompiler<T extends TransformCompilerType, TranBase extends Tran> extends SectionCompiler {
static validateMarkers(keyboard: LDMLKeyboard.LKKeyboard, emitMarkers: Set<string>, matchMarkers: Set<string>): boolean {
static validateMarkers(keyboard: LDMLKeyboard.LKKeyboard, mt : MarkerTracker): boolean {
keyboard?.transforms?.forEach(transforms =>
transforms.transformGroup.forEach(transformGroup => {
transformGroup.transform?.forEach(({ to, from }) => {
MarkerParser.allReferences(from).forEach(marker => matchMarkers.add(marker));
MarkerParser.allReferences(to).forEach(marker => emitMarkers.add(marker));
});
}));
mt.add(MarkerUse.emit, MarkerParser.allReferences(to));
mt.add(MarkerUse.consume, MarkerParser.allReferences(from));
})}));
return true;
}

View file

@ -12,6 +12,7 @@ import { CompilerMessages } from "./messages.js";
import { KeysCompiler } from "./keys.js";
import { TransformCompiler } from "./tran.js";
import { DispCompiler } from "./disp.js";
import { MarkerTracker, MarkerUse } from "./marker-tracker.js";
export class VarsCompiler extends SectionCompiler {
public get id() {
return constants.section.vars;
@ -20,7 +21,8 @@ export class VarsCompiler extends SectionCompiler {
public get dependencies(): Set<SectionIdent> {
const defaults = new Set(<SectionIdent[]>[
constants.section.strs,
constants.section.elem
constants.section.elem,
constants.section.list,
]);
defaults.delete(this.id);
return defaults;
@ -32,7 +34,6 @@ export class VarsCompiler extends SectionCompiler {
public validate(): boolean {
let valid = true;
// TODO-LDML scan for markers?
// Check for duplicate ids
const allIds = new Set();
@ -139,51 +140,47 @@ export class VarsCompiler extends SectionCompiler {
return valid;
}
private collectMarkers(emitMarkers : Set<string>, matchMarkers : Set<string>) : boolean {
private collectMarkers(mt : MarkerTracker) : boolean {
let valid = true;
// call our friends to validate
valid = this.validateVarsMarkers(this.keyboard, emitMarkers, matchMarkers) && valid; // accumulate validity
valid = KeysCompiler.validateMarkers(this.keyboard, emitMarkers, matchMarkers) && valid; // accumulate validity
valid = TransformCompiler.validateMarkers(this.keyboard, emitMarkers, matchMarkers) && valid; // accumulate validity
valid = DispCompiler.validateMarkers(this.keyboard, emitMarkers, matchMarkers) && valid; // accumulate validity
valid = this.validateVarsMarkers(this.keyboard, mt) && valid; // accumulate validity
valid = KeysCompiler.validateMarkers(this.keyboard, mt) && valid; // accumulate validity
valid = TransformCompiler.validateMarkers(this.keyboard, mt) && valid; // accumulate validity
valid = DispCompiler.validateMarkers(this.keyboard, mt) && valid; // accumulate validity
return valid;
}
private validateMarkers(): boolean {
/** only the markers used in emitters */
const emitMarkers : Set<string> = new Set<string>();
/** only the markers used in matchers */
const matchMarkers : Set<string> = new Set<string>();
let valid = this.collectMarkers(emitMarkers, matchMarkers);
const mt = new MarkerTracker();
let valid = this.collectMarkers(mt);
// see if there are any matched-but-not-emitted
const matchedNotEmitted : string[] = [];
for (const m of matchMarkers.values()) {
if (m === '.') continue; // match-all marker
if (!emitMarkers.has(m)) {
matchedNotEmitted.push(m);
const matchedNotEmitted : Set<string> = new Set<string>();
for (const m of mt.matched.values()) {
if (m === MarkerParser.ANY_MARKER_ID) continue; // match-all marker
if (!mt.emitted.has(m)) {
matchedNotEmitted.add(m);
}
}
for (const m of mt.consumed.values()) {
if (m === MarkerParser.ANY_MARKER_ID) continue; // match-all marker
if (!mt.emitted.has(m)) {
matchedNotEmitted.add(m);
}
}
// report once
if (matchedNotEmitted.length) {
matchedNotEmitted.sort();
this.callbacks.reportMessage(CompilerMessages.Error_MissingMarkers({ ids: matchedNotEmitted }));
if (matchedNotEmitted.size > 0) {
this.callbacks.reportMessage(CompilerMessages.Error_MissingMarkers({ ids: Array.from(matchedNotEmitted.values()).sort() }));
valid = false;
}
return valid;
}
validateVarsMarkers(keyboard: LDMLKeyboard.LKKeyboard, emitMarkers: Set<string>, matchMarkers: Set<string>) : boolean {
validateVarsMarkers(keyboard: LDMLKeyboard.LKKeyboard, mt : MarkerTracker) : boolean {
keyboard?.variables?.string?.forEach(({value}) =>
MarkerParser.allReferences(value).forEach(marker => {
emitMarkers.add(marker);
matchMarkers.add(marker);
}));
mt.add(MarkerUse.variable, MarkerParser.allReferences(value)));
return true;
}
@ -207,6 +204,14 @@ export class VarsCompiler extends SectionCompiler {
variables?.unicodeSet?.forEach((e) =>
this.addUnicodeSet(result, e, sections));
// reload markers - TODO-LDML: double work!
const mt = new MarkerTracker();
this.collectMarkers(mt);
// collect all markers, excluding the match-all
const allMarkers : string[] = Array.from(mt.all).filter(m => m !== MarkerParser.ANY_MARKER_ID).sort();
result.markers = sections.list.allocList(sections.strs, allMarkers);
return result.valid() ? result : null;
}

View file

@ -15,30 +15,30 @@
</names>
<displays>
<display to="\{m}" display="Ⓜ️" />
<display to="\m{m}" display="Ⓜ️" />
</displays>
<keys>
<key id="m" to="\{m}" />
<key id="m" to="\m{m}" />
</keys>
<!-- from spec -->
<variables>
<string id="x" value="\{x}" />
<string id="x" value="\m{x}" />
</variables>
<transforms type="simple">
<transformGroup>
<transform from="m" to="\{m}"/>
<transform from="x" to="\{x}"/>
<transform from="m" to="\m{m}"/>
<transform from="x" to="\m{x}"/>
</transformGroup>
<transformGroup>
<transform from="e\{x}" to="é" />
<transform from="e\m{x}" to="é" />
</transformGroup>
</transforms>
<transforms type="backspace">
<transformGroup>
<transform from="A\{.}B" />
<transform from="A\m{.}B" />
</transformGroup>
</transforms>
</keyboard>

View file

@ -189,6 +189,12 @@ describe('vars', function () {
testCompilationCases(VarsCompiler, [
{
subpath: 'sections/vars/markers-maximal.xml',
callback(sect) {
const vars = <Vars> sect;
assert.ok(vars.markers);
assert.sameDeepOrderedMembers(vars.markers.toStringArray(),
['m','x']);
},
},
{
subpath: 'sections/vars/fail-markers-badref-0.xml',