Skip to content

Commit

Permalink
Merge pull request #12588 from keymanapp/feat/developer/12505-report-…
Browse files Browse the repository at this point in the history 
…key-address-in-layout-compiler

feat(developer): Report key 'address' in validation failures in layout compiler
  • Loading branch information
@mcdurdin
mcdurdin authored Nov 7, 2024
2 parents 08215e2 + 6002024 commit 78999fb
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 25 deletions.
33 changes: 22 additions & 11 deletions developer/src/kmc-kmn/src/compiler/kmn-compiler-messages.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { KeyAddress } from "../kmw-compiler/validate-layout-file.js";
import { kmnfile } from "../kmw-compiler/compiler-globals.js";
import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerEvent, CompilerMessageSpec as m, CompilerMessageDef as def, CompilerMessageSpecWithException, KeymanUrls } from "@keymanapp/developer-utils";

Expand Down Expand Up @@ -26,6 +27,17 @@ export const enum KmnCompilerMessageRanges {
RANGE_CompilerMessage_Max = 0xFFF, // Highest available error code for kmc-kmn
}

export function keyAddress(address: KeyAddress): string {
if(!address) {
return '<param>';
}
return `row:${address.rowIndex} col:${address.keyIndex}` +
(address.direction ? ` flick:${address.direction}` :
(address.subKeyIndex ? ` longPress:${address.subKeyIndex}` :
(address.multitapIndex ? ` multitap:${address.multitapIndex}` :
'')));
}

type KmcmpLibMessageParameters = {p:string[]};

/**
Expand Down Expand Up @@ -451,8 +463,8 @@ export class KmnCompilerMessages {
`Touch layout file ${def(o.filename)} is not valid`);

static ERROR_TouchLayoutInvalidIdentifier = SevError | 0x05A;
static Error_TouchLayoutInvalidIdentifier = (o:{keyId:string, platformName: string, layerId:string}) => mw(this.ERROR_TouchLayoutInvalidIdentifier,
`Key "${def(o.keyId)}" on "${def(o.platformName)}", layer "${def(o.layerId)}" has an invalid identifier.`);
static Error_TouchLayoutInvalidIdentifier = (o:{keyId:string, platformName: string, layerId:string, address:KeyAddress}) => mw(this.ERROR_TouchLayoutInvalidIdentifier,
`Key "${def(o.keyId)}" on "${def(o.platformName)}", layer "${def(o.layerId)}" (${keyAddress(o.address)}) has an invalid identifier.`);

static ERROR_InvalidKeyCode = SevError | 0x05B;
static Error_InvalidKeyCode = (o:{keyId: string}) => mw(this.ERROR_InvalidKeyCode,
Expand Down Expand Up @@ -619,14 +631,13 @@ export class KmnCompilerMessages {
static WARN_PunctuationInEthnologueCode = SevWarn | 0x090;
static Warn_PunctuationInEthnologueCode = () => m(this.WARN_PunctuationInEthnologueCode, `Punctuation should not be used to separate Ethnologue codes; instead use spaces`);


static WARN_TouchLayoutMissingLayer = SevWarn | 0x091;
static Warn_TouchLayoutMissingLayer = (o:{keyId:string, platformName:string, layerId:string, nextLayer:string}) => mw(this.WARN_TouchLayoutMissingLayer,
`Key "${def(o.keyId)}" on platform "${def(o.platformName)}", layer "${def(o.layerId)}", references a missing layer "${def(o.nextLayer)}"`);
static Warn_TouchLayoutMissingLayer = (o:{keyId:string, platformName:string, layerId:string, nextLayer:string, address:KeyAddress}) => mw(this.WARN_TouchLayoutMissingLayer,
`Key "${def(o.keyId)}" on platform "${def(o.platformName)}", layer "${def(o.layerId)}" (${keyAddress(o.address)}), references a missing layer "${def(o.nextLayer)}"`);

static WARN_TouchLayoutCustomKeyNotDefined = SevWarn | 0x092;
static Warn_TouchLayoutCustomKeyNotDefined = (o:{keyId:string, platformName:string, layerId:string}) => mw(this.WARN_TouchLayoutCustomKeyNotDefined,
`Key "${def(o.keyId)}" on platform "${def(o.platformName)}", layer "${def(o.layerId)}", is a custom key but has no corresponding rule in the source.`);
static Warn_TouchLayoutCustomKeyNotDefined = (o:{keyId:string, platformName:string, layerId:string, address:KeyAddress}) => mw(this.WARN_TouchLayoutCustomKeyNotDefined,
`Key "${def(o.keyId)}" on platform "${def(o.platformName)}", layer "${def(o.layerId)}" (${keyAddress(o.address)}), is a custom key but has no corresponding rule in the source.`);

static WARN_TouchLayoutMissingRequiredKeys = SevWarn | 0x093;
static Warn_TouchLayoutMissingRequiredKeys = (o:{layerId:string, platformName:string, missingKeys:string}) => mw(this.WARN_TouchLayoutMissingRequiredKeys,
Expand All @@ -648,8 +659,8 @@ export class KmnCompilerMessages {
`Extended shift flags ${def(o.flags)} are not supported in KeymanWeb`, o);

static WARN_TouchLayoutUnidentifiedKey = SevWarn | 0x099;
static Warn_TouchLayoutUnidentifiedKey = (o:{layerId:string}) => mw(this.WARN_TouchLayoutUnidentifiedKey,
`A key on layer "${def(o.layerId)}" has no identifier.`);
static Warn_TouchLayoutUnidentifiedKey = (o:{layerId:string, address:KeyAddress}) => mw(this.WARN_TouchLayoutUnidentifiedKey,
`A key (${keyAddress(o.address)}) on layer "${def(o.layerId)}" has no identifier.`);

static HINT_UnreachableKeyCode = SevHint | 0x09A;
static Hint_UnreachableKeyCode = (o:{line:number,key:string}) => mw(this.HINT_UnreachableKeyCode,
Expand Down Expand Up @@ -696,9 +707,9 @@ export class KmnCompilerMessages {
static Warn_HotkeyHasInvalidModifier = () => m(this.WARN_HotkeyHasInvalidModifier, `Hotkey has modifiers that are not supported. Use only SHIFT, CTRL and ALT`);

static WARN_TouchLayoutSpecialLabelOnNormalKey = SevWarn | 0x0A9;
static Warn_TouchLayoutSpecialLabelOnNormalKey = (o:{keyId:string, platformName:string, layerId:string, label:string}) =>
static Warn_TouchLayoutSpecialLabelOnNormalKey = (o:{keyId:string, platformName:string, layerId:string, label:string, address:KeyAddress}) =>
mw(this.WARN_TouchLayoutSpecialLabelOnNormalKey,
`Key "${def(o.keyId)}" on platform "${def(o.platformName)}", layer "${def(o.layerId)}" does not have `+
`Key "${def(o.keyId)}" on platform "${def(o.platformName)}", layer "${def(o.layerId)}" (${keyAddress(o.address)}) does not have `+
`the key type "Special" or "Special (active)" but has the label "${def(o.label)}". This feature is only supported in Keyman 14 or later`);

static WARN_OptionStoreNameInvalid = SevWarn | 0x0AA;
Expand Down
7 changes: 4 additions & 3 deletions developer/src/kmc-kmn/src/kmw-compiler/kmw-compiler-messages.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { KmnCompilerMessages } from "../compiler/kmn-compiler-messages.js";
import { keyAddress, KmnCompilerMessages } from "../compiler/kmn-compiler-messages.js";
import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerEvent, CompilerMessageDef as def, CompilerMessageSpec } from "@keymanapp/developer-utils";
import { kmnfile } from "./compiler-globals.js";
import { KeyAddress } from "./validate-layout-file.js";

const Namespace = CompilerErrorNamespace.KmwCompiler;
const SevInfo = CompilerErrorSeverity.Info | Namespace;
Expand Down Expand Up @@ -34,8 +35,8 @@ export class KmwCompilerMessages extends KmnCompilerMessages {
// SevError | 0x0001, added in 17.0, removed in 18.0

static ERROR_TouchLayoutIdentifierRequires15 = SevError | 0x0002;
static Error_TouchLayoutIdentifierRequires15 = (o:{keyId:string, platformName:string, layerId:string}) => m(this.ERROR_TouchLayoutIdentifierRequires15,
`Key "${def(o.keyId)}" on "${def(o.platformName)}", layer "${def(o.layerId)}" has a multi-part identifier which requires version 15.0 or newer.`);
static Error_TouchLayoutIdentifierRequires15 = (o:{keyId:string, platformName:string, layerId:string, address:KeyAddress}) => m(this.ERROR_TouchLayoutIdentifierRequires15,
`Key "${def(o.keyId)}" on "${def(o.platformName)}", layer "${def(o.layerId)}" (${keyAddress(o.address)}) has a multi-part identifier which requires version 15.0 or newer.`);

static ERROR_InvalidTouchLayoutFileFormat = SevError | 0x0003;
static Error_InvalidTouchLayoutFileFormat = (o:{msg: string}) => m(this.ERROR_InvalidTouchLayoutFileFormat,
Expand Down
43 changes: 32 additions & 11 deletions developer/src/kmc-kmn/src/kmw-compiler/validate-layout-file.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ import { KeymanWebTouchStandardKeyNames, KMWAdditionalKeyNames, VKeyNames } from
import { KmwCompilerMessages } from "./kmw-compiler-messages.js";
import * as Osk from '../compiler/osk.js';

export interface KeyAddress {
rowIndex: number;
keyIndex: number;
subKeyIndex?: number;
direction?: string;
multitapIndex?: number
};

interface VLFOutput {
output: string;
result: boolean;
Expand Down Expand Up @@ -72,7 +80,8 @@ function CheckKey(
FNextLayer: string,
FKeyType: TouchLayout.TouchLayoutKeySp,
FRequiredKeys: TRequiredKey[],
FDictionary: string[]
FDictionary: string[],
address: KeyAddress
): boolean { // I4119
//
// Coerce missing ID and Text to empty strings for additional tests
Expand Down Expand Up @@ -104,7 +113,8 @@ function CheckKey(
keyId: FId,
layerId: layer.id,
nextLayer: FNextLayer,
platformName: platformId
platformName: platformId,
address
}));
}
}
Expand All @@ -115,19 +125,19 @@ function CheckKey(

if(FId.trim() == '') {
if(!([TouchLayout.TouchLayoutKeySp.blank, TouchLayout.TouchLayoutKeySp.spacer].includes(FKeyType))) {
callbacks.reportMessage(KmwCompilerMessages.Warn_TouchLayoutUnidentifiedKey({layerId: layer.id}));
callbacks.reportMessage(KmwCompilerMessages.Warn_TouchLayoutUnidentifiedKey({layerId: layer.id, address}));
}
return true;
}

let FValid = KeyIdType(FId);

if(FValid == TKeyIdType.Key_Invalid) {
callbacks.reportMessage(KmwCompilerMessages.Error_TouchLayoutInvalidIdentifier({keyId: FId, platformName: platformId, layerId: layer.id}));
callbacks.reportMessage(KmwCompilerMessages.Error_TouchLayoutInvalidIdentifier({keyId: FId, platformName: platformId, layerId: layer.id, address}));
return false;
}
else if (FValid == TKeyIdType.Key_Unicode_Multi && !verifyAndSetMinimumRequiredKeymanVersion15()) {
callbacks.reportMessage(KmwCompilerMessages.Error_TouchLayoutIdentifierRequires15({keyId: FId, platformName: platformId, layerId: layer.id}));
callbacks.reportMessage(KmwCompilerMessages.Error_TouchLayoutIdentifierRequires15({keyId: FId, platformName: platformId, layerId: layer.id, address}));
return false;
}

Expand All @@ -138,7 +148,7 @@ function CheckKey(
if (FValid == TKeyIdType.Key_Touch && FNextLayer == '' && [TouchLayout.TouchLayoutKeySp.normal, TouchLayout.TouchLayoutKeySp.deadkey].includes(FKeyType)) {
// Search for the key in the key dictionary - ignore K_LOPT, K_ROPT...
if(FDictionary.indexOf(FId) < 0) {
callbacks.reportMessage(KmwCompilerMessages.Warn_TouchLayoutCustomKeyNotDefined({keyId: FId, platformName: platformId, layerId: layer.id}));
callbacks.reportMessage(KmwCompilerMessages.Warn_TouchLayoutCustomKeyNotDefined({keyId: FId, platformName: platformId, layerId: layer.id, address}));
}
}

Expand All @@ -158,7 +168,8 @@ function CheckKey(
keyId: FId,
platformName: platformId,
layerId: layer.id,
label: FText
label: FText,
address
}));
}
}
Expand Down Expand Up @@ -269,27 +280,37 @@ export function ValidateLayoutFile(fk: KMX.KEYBOARD, FDebug: boolean, sLayoutFil
// Test that all required keys are present
for(let layer of platform.layer) {
let FRequiredKeys: TRequiredKey[] = [];
let rowIndex = 0;
for(let row of layer.row) {
rowIndex++;
let keyIndex = 0;
for(let key of row.key) {
result = CheckKey(pid, platform, layer, key.id, key.text, key.nextlayer, key.sp, FRequiredKeys, FDictionary) && result; // I4119
keyIndex++;
result = CheckKey(pid, platform, layer, key.id, key.text, key.nextlayer, key.sp, FRequiredKeys, FDictionary, {rowIndex, keyIndex}) && result; // I4119
if(key.sk) {
let subKeyIndex = 0;
for(let subkey of key.sk) {
result = CheckKey(pid, platform, layer, subkey.id, subkey.text, subkey.nextlayer, subkey.sp, FRequiredKeys, FDictionary) && result;
subKeyIndex++;
result = CheckKey(pid, platform, layer, subkey.id, subkey.text, subkey.nextlayer, subkey.sp, FRequiredKeys, FDictionary,
{rowIndex, keyIndex, subKeyIndex}) && result;
}
}
let direction: keyof TouchLayout.TouchLayoutFlick;
if(key.flick) {
for(direction in key.flick) {
warnGesturesIfNeeded(key.id);
result = CheckKey(pid, platform, layer, key.flick[direction].id, key.flick[direction].text,
key.flick[direction].nextlayer, key.flick[direction].sp, FRequiredKeys, FDictionary) && result;
key.flick[direction].nextlayer, key.flick[direction].sp, FRequiredKeys, FDictionary, {rowIndex, keyIndex, direction}) && result;
}
}

if(key.multitap) {
let multitapIndex = 0;
for(let subkey of key.multitap) {
multitapIndex++;
warnGesturesIfNeeded(key.id);
result = CheckKey(pid, platform, layer, subkey.id, subkey.text, subkey.nextlayer, subkey.sp, FRequiredKeys, FDictionary) && result;
result = CheckKey(pid, platform, layer, subkey.id, subkey.text, subkey.nextlayer, subkey.sp, FRequiredKeys, FDictionary,
{rowIndex, keyIndex, multitapIndex}) && result;
}
}
}
Expand Down

0 comments on commit 78999fb

Please sign in to comment.