Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
19ebc62
feat(developer): ensure typesafety for 'return null' and others
SabineSIL Apr 20, 2026
30ea904
feat(developer): remove tests for entire keyboards
SabineSIL Apr 20, 2026
6802ec4
feat(developer): read() throw error instead of return null
SabineSIL Apr 20, 2026
9b9cafe
feat(developer): more typesafety + add tests for validate()
SabineSIL Apr 21, 2026
ef1cf18
feat(developer): more typesafety
SabineSIL Apr 21, 2026
417c044
feat(developer): baseUrl problem of tsconfig.json, errorMsg typo
SabineSIL Apr 21, 2026
99892e3
Merge branch 'feat/developer/kmc-convert' into feat/developer/kmc-con…
SabineSIL Apr 21, 2026
4716887
feat(developer): restore tsconfig.json
SabineSIL Apr 21, 2026
2edf63c
Merge branch 'feat/developer/kmc-convert-Typesafety' of https://githu…
SabineSIL Apr 21, 2026
16e0570
Merge branch 'feat/developer/kmc-convert' into feat/developer/kmc-con…
SabineSIL May 4, 2026
1d3d53e
feat(developer): add returntypes
SabineSIL May 4, 2026
9675611
feat(developer): add tests and comments
SabineSIL May 5, 2026
302e7b0
feat(developer): add testfile
SabineSIL May 5, 2026
12f2d19
Merge branch 'feat/developer/kmc-convert' into feat/developer/kmc-con…
SabineSIL May 19, 2026
eb5ac1e
Merge branch 'feat/developer/kmc-convert' into feat/developer/kmc-con…
SabineSIL May 20, 2026
5052581
feat(developer): add missing bracket
SabineSIL May 20, 2026
b72e98e
Merge branch 'feat/developer/kmc-convert' into feat/developer/kmc-con…
SabineSIL May 20, 2026
82c8af2
feat(developer): dummy change to start team city
SabineSIL May 20, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion developer/src/kmc-convert/src/converter-messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const SevError = CompilerErrorSeverity.Error | Namespace;
export class ConverterMessages {

static ERROR_FileNotFound = SevError | 0x0003;
static Error_FileNotFound = (o: { inputFilename: string; }) => m(
static Error_FileNotFound = (o: { inputFilename: string | null; }) => m(
this.ERROR_FileNotFound,
`Input filename '${def(o.inputFilename)}' does not exist or could not be loaded.`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you allow inputFilename to be null, then this has to be:

Suggested change
`Input filename '${def(o.inputFilename)}' does not exist or could not be loaded.`
`Input filename '${def(o?.inputFilename)}' does not exist or could not be loaded.`

But I think better would be not to allow null for inputFilename.

);
Expand Down
4 changes: 2 additions & 2 deletions developer/src/kmc-convert/src/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ export class Converter implements KeymanCompiler {
return null;
}

const ConverterClass = ConverterClassFactory.find(inputFilename, outputFilename);
const ConverterClass = ConverterClassFactory.find(inputFilename, outputFilename ?? '.kmn');
if (!ConverterClass) {
this.callbacks.reportMessage(ConverterMessages.Error_NoConverterFound({ inputFilename, outputFilename }));
this.callbacks.reportMessage(ConverterMessages.Error_NoConverterFound({ inputFilename, outputFilename: outputFilename ?? '' }));
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ export class KeylayoutFileReader {

constructor(private callbacks: CompilerCallbacks /*,private options: CompilerOptions*/) { };


/**
* @brief helper function to find a specific keyMap index in a keyMapSet
* @brief helper function to check if a specific keyMap index exists in a keyMapSet
* neccessary because the amount of <keyMap index> must correspond to
* the amount of <keyMapSelect mapIndex>
Comment on lines +21 to +23
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to make this full sentences. If this gets processed by a tool the three lines might get put in the same paragraph removing the line breaks, which then would make it hard to understand.

Suggested change
* @brief helper function to check if a specific keyMap index exists in a keyMapSet
* neccessary because the amount of <keyMap index> must correspond to
* the amount of <keyMapSelect mapIndex>
* @brief Helper function to check if a specific keyMap index exists in a keyMapSet.
* This is neccessary because the amount of <keyMap index> must correspond to
* the amount of <keyMapSelect mapIndex>.

* @param jsonObj the read keylayout data to be checked
* @param keyMapSelect the keyMapSelect element to find in keyMapSet
* @return true if the keyMapSet element is found, false if not
Expand All @@ -36,7 +37,9 @@ export class KeylayoutFileReader {
}

/**
* @brief helper function to find a specific keyMapSelect index in a modifierMap
* @brief helper function to check if a specific keyMapSelect index exists in a modifierMap
* neccessary because the amount of <keyMap index> must correspond to
* the amount of <keyMapSelect mapIndex>
Comment on lines +40 to +42
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @brief helper function to check if a specific keyMapSelect index exists in a modifierMap
* neccessary because the amount of <keyMap index> must correspond to
* the amount of <keyMapSelect mapIndex>
* @brief Helper function to check if a specific keyMapSelect index exists in a modifierMap.
* This is neccessary because the amount of <keyMap index> must correspond to
* the amount of <keyMapSelect mapIndex>.

* @param jsonObj the read keylayout data to be checked
* @param keyMap the keyMap element to find in modifierMap
* @return true if the keyMap element is found, false if not
Expand All @@ -53,8 +56,8 @@ export class KeylayoutFileReader {
}

/**
* @brief member function checking if all keyMapSelect elements have a corresponding keyMap
* element in the .keylayout file (if not, the .keylayout file is invalid and will not be converted)
* @brief member function checking if all keyMapSelect elements have exact one corresponding keyMap element (per keyMapSet)
* in the .keylayout file (if not, the .keylayout file is invalid and will not be converted)
* see TN2056 (https://developer.apple.com/library/archive/technotes/tn2056/_index.html#//apple_ref/doc/uid/DTS10003085-CH1-SUBSECTION7)
* @param jsonObj the read keylayout data to be checked
* @return true if all keyMapSelect elements have a corresponding keyMap element, false if not
Expand Down Expand Up @@ -82,6 +85,10 @@ export class KeylayoutFileReader {
* @returns true if valid, false if invalid
*/
public validate(source: Keylayout.KeylayoutXMLSourceFile, inputFilename: string): boolean {
if (!source) {
this.callbacks.reportMessage(ConverterMessages.Error_UnableToReadFile({ inputFilename: inputFilename }));
return false;
}
if (!SchemaValidators.default.keylayout(source)) {
for (const err of (<any>SchemaValidators.default.keylayout).errors) {
this.callbacks.reportMessage(DeveloperUtilsMessages.Error_InvalidXml({
Expand Down Expand Up @@ -148,7 +155,7 @@ export class KeylayoutFileReader {
* @param inputFilename the ukelele .keylayout-file to be parsed
* @return in case of success: json object containing data of the .keylayout file; else null
*/
public read(source: Uint8Array): Keylayout.KeylayoutXMLSourceFile {
public read(source: Uint8Array): Keylayout.KeylayoutXMLSourceFile | null {

try {
const data = new TextDecoder().decode(source);
Expand Down

Large diffs are not rendered by default.

76 changes: 43 additions & 33 deletions developer/src/kmc-convert/src/keylayout-to-kmn/kmn-file-writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import { CompilerCallbacks, CompilerOptions } from "@keymanapp/developer-utils";
import { KeylayoutToKmnConverter, ProcessedData, Rule } from './keylayout-to-kmn-converter.js';
import { ConverterMessages } from '../converter-messages.js';
import KEYMAN_VERSION from "@keymanapp/keyman-version";

interface MessageCharacter {
Expand Down Expand Up @@ -47,20 +46,18 @@ export class KmnFileWriter {
if (dataRules)
data += dataStores + dataRules;

try {
return new TextEncoder().encode(data);
} catch (err) {
this.callbacks.reportMessage(ConverterMessages.Error_UnableToWrite({ outputFilename: dataUkelele.kmnFilename, errorText: err }));
return null;
}
return new TextEncoder().encode(data);
}

/**
* @brief member function to create data for the header (stores) that will be printed to the resulting kmn file
* @param dataUkelele an object containing all data read from a .keylayout file
* @return string - all stores to be printed
*/
public writeKmnFileHeader(dataUkelele: ProcessedData): string {
public writeKmnFileHeader(dataUkelele: ProcessedData | null): string {
if (!dataUkelele) {
return "";
}

let data: string = "";

Expand All @@ -87,8 +84,10 @@ export class KmnFileWriter {
* @param dataUkelele an object containing all data read from a .keylayout file
* @return string - all rules to be printed
*/
public writeDataRules(dataUkelele: ProcessedData): string {

public writeDataRules(dataUkelele: ProcessedData | null): string {
if (!dataUkelele) {
return "";
}
const keylayoutKmnConverter = new KeylayoutToKmnConverter(this.callbacks, this.options);
let data: string = "";

Expand All @@ -104,7 +103,7 @@ export class KmnFileWriter {
|| (curr.ruleType === "C2" && (curr.deadkey !== ""))
|| (curr.ruleType === "C3" && (curr.deadkey !== "") && (curr.prevDeadkey !== "")))
);
}).reduce((unique, o) => {
}).reduce<Rule[]>((unique, o) => {
if (!unique.some((obj: Rule) =>
new TextDecoder().decode(obj.output) === new TextDecoder().decode(o.output)

Expand All @@ -121,7 +120,7 @@ export class KmnFileWriter {
unique.push(o);
}
return unique;
}, []);
}, [] as Rule[]);

//................................................ C0 C1 ................................................................

Expand Down Expand Up @@ -158,10 +157,15 @@ export class KmnFileWriter {
// const outputUnicodeCharacter = util.convertToUnicodeCharacter(outputCharacter);
// const outputUnicodeCodePoint = util.convertToUnicodeCodePoint(outputCharacter);

if ((outputCharacter !== undefined) || (outputCharacter !== "")) {
// in case writeCharacterOrUnicode() returns null, the fallback is empty strings for characterMessage.character
// and characterMessage.message. Then versionOutputCharacter could be "" and would be written into the kmn file
// as ... > '', producing an invalid kmn rule.
if ((outputCharacter !== undefined) && (outputCharacter !== "")) {
const characterMessage = this.writeCharacterOrUnicode(outputCharacter, warnText[2]);
versionOutputCharacter = characterMessage.character;
warnText[2] = characterMessage.message;

versionOutputCharacter = characterMessage?.character ?? "";
warnText[2] = characterMessage?.message ?? "";

}

// add a warning in front of rules in case unavailable modifiers or ambiguous rules are used
Expand Down Expand Up @@ -223,10 +227,13 @@ export class KmnFileWriter {
// const outputUnicodeCharacter = util.convertToUnicodeCharacter(outputCharacter);
// const outputUnicodeCodePoint = util.convertToUnicodeCodePoint(outputCharacter);

if ((outputCharacter !== undefined) || (outputCharacter !== "")) {
// in case writeCharacterOrUnicode() returns null, the fallback is empty strings for characterMessage.character
// and characterMessage.message. Then versionOutputCharacter could be "" and would be written into the kmn file
// as ... > '', producing an invalid kmn rule.
if ((outputCharacter !== undefined) && (outputCharacter !== "")) {
const characterMessage = this.writeCharacterOrUnicode(outputCharacter, warnText[2]);
versionOutputCharacter = characterMessage.character;
warnText[2] = characterMessage.message;
versionOutputCharacter = characterMessage?.character ?? "";
warnText[2] = characterMessage?.message ?? "";
}

// add a warning in front of rules in case unavailable modifiers or ambiguous rules are used
Expand Down Expand Up @@ -308,10 +315,13 @@ export class KmnFileWriter {
const outputCharacter = new TextDecoder().decode(uniqueDataRules[k].output);
// TODO-kmc-convert: after merge of PR 14569 use functions from util instead of the ones in this class

if ((outputCharacter !== undefined) || (outputCharacter !== "")) {
// in case writeCharacterOrUnicode() returns null, the fallback is empty strings for characterMessage.character
// and characterMessage.message. Then versionOutputCharacter could be "" and would be written into the kmn file
// as ... > '', producing an invalid kmn rule.
Comment on lines +318 to +320
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this comment. It describes what the code does and that this could result in an invalid kmn rule. Wouldn't we want to do something so that we don't get invalid kmn rules? (in which case I'd expect a TODO or something).

Or does the comment try to explain why we have the if ((outputCharacter !== undefined) && (outputCharacter !== "")) check? Then it should mention outputCharacter which could lead to writeCharacterOrUnicode returning false...

(Additional occurrences of the same code comment above)

if ((outputCharacter !== undefined) && (outputCharacter !== "")) {
const characterMessage = this.writeCharacterOrUnicode(outputCharacter, warnText[2]);
versionOutputCharacter = characterMessage.character;
warnText[2] = characterMessage.message;
versionOutputCharacter = characterMessage?.character ?? "";
warnText[2] = characterMessage?.message ?? "";
}

// add a warning in front of rules in case unavailable modifiers or ambiguous rules are used
Expand Down Expand Up @@ -533,7 +543,7 @@ export class KmnFileWriter {
+ " "
+ amb_1_1[0].key
+ "] > \'"
+ this.writeCharacterOrUnicode(new TextDecoder().decode(amb_1_1[0].output)).character
+ (this.writeCharacterOrUnicode(new TextDecoder().decode(amb_1_1[0].output))?.character ?? "")
+ "\' ");
}

Expand All @@ -544,7 +554,7 @@ export class KmnFileWriter {
+ " "
+ dup_1_1[0].key
+ "] > \'"
+ this.writeCharacterOrUnicode(new TextDecoder().decode(dup_1_1[0].output)).character
+ (this.writeCharacterOrUnicode(new TextDecoder().decode(dup_1_1[0].output))?.character ?? "")
+ "\' ");
}
}
Expand Down Expand Up @@ -629,7 +639,7 @@ export class KmnFileWriter {
+ " "
+ amb_3_3[0].key
+ "] > \'"
+ this.writeCharacterOrUnicode(new TextDecoder().decode(amb_3_3[0].output)).character
+ (this.writeCharacterOrUnicode(new TextDecoder().decode(amb_3_3[0].output))?.character ?? "")
+ "\' ");
}

Expand All @@ -642,7 +652,7 @@ export class KmnFileWriter {
+ " "
+ dup_3_3[0].key
+ "] > \'"
+ this.writeCharacterOrUnicode(new TextDecoder().decode(dup_3_3[0].output)).character
+ (this.writeCharacterOrUnicode(new TextDecoder().decode(dup_3_3[0].output))?.character ?? "")
+ "\' ");
}

Expand Down Expand Up @@ -770,7 +780,7 @@ export class KmnFileWriter {
+ " "
+ amb_6_3[0].key
+ "] > \'"
+ this.writeCharacterOrUnicode(new TextDecoder().decode(amb_6_3[0].output)).character
+ (this.writeCharacterOrUnicode(new TextDecoder().decode(amb_6_3[0].output))?.character ?? "")
+ "\' ");
}

Expand All @@ -783,7 +793,7 @@ export class KmnFileWriter {
+ " "
+ dup_6_3[0].key
+ "] > \'"
+ this.writeCharacterOrUnicode(new TextDecoder().decode(dup_6_3[0].output)).character
+ (this.writeCharacterOrUnicode(new TextDecoder().decode(dup_6_3[0].output))?.character ?? "")
+ "\' ");
}

Expand Down Expand Up @@ -844,7 +854,7 @@ export class KmnFileWriter {
+ " "
+ amb_6_6[0].key
+ "] > \'"
+ this.writeCharacterOrUnicode(new TextDecoder().decode(amb_6_6[0].output)).character
+ (this.writeCharacterOrUnicode(new TextDecoder().decode(amb_6_6[0].output))?.character ?? "")
+ "\' ");
}

Expand All @@ -857,7 +867,7 @@ export class KmnFileWriter {
+ " "
+ dup_6_6[0].key
+ "] > \'"
+ this.writeCharacterOrUnicode(new TextDecoder().decode(dup_6_6[0].output)).character
+ (this.writeCharacterOrUnicode(new TextDecoder().decode(dup_6_6[0].output))?.character ?? "")
+ "\' ");
}
}
Expand Down Expand Up @@ -1626,7 +1636,7 @@ export class KmnFileWriter {
* a non-control character will be written as itself ( 'A', '1', '፩', '😎')
* null in case of an empty string or null or undefined input
*/
public writeCharacterOrUnicode(ctr: string, msg: string = ""): MessageCharacter {
public writeCharacterOrUnicode(ctr: string, msg: string = ""): MessageCharacter | null {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better if writeCharacterToUnicode would always returns MessageCharacter. In the error case it could return { character: '', message: '' }. IMO this would simplify things quite a bit.


if ((ctr === null) || (ctr === undefined) || (ctr.length === 0)) {
return null;
Expand All @@ -1644,7 +1654,7 @@ export class KmnFileWriter {

// find the value of output character which may be specified in unicode, html hex or html dec format ( e.g. U+1234 -> 1234; &#x1234; -> 1234; &#4660; -> 1234)
const ctr_val = ((m_uni || m_hex || m_dec) ?
m_uni ? parseInt(m_uni[1], 16) : m_hex ? parseInt(m_hex[1], 16) : parseInt(m_dec[1], 10) : KeylayoutToKmnConverter.MAX_CTRL_CHARACTER
m_uni ? parseInt(m_uni[1], 16) : m_hex ? parseInt(m_hex[1], 16) : m_dec ? parseInt(m_dec[1], 10) : KeylayoutToKmnConverter.MAX_CTRL_CHARACTER : KeylayoutToKmnConverter.MAX_CTRL_CHARACTER
Comment on lines 1656 to +1657
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this multi-line since it's so many nested ternary operators:

Suggested change
const ctr_val = ((m_uni || m_hex || m_dec) ?
m_uni ? parseInt(m_uni[1], 16) : m_hex ? parseInt(m_hex[1], 16) : parseInt(m_dec[1], 10) : KeylayoutToKmnConverter.MAX_CTRL_CHARACTER
m_uni ? parseInt(m_uni[1], 16) : m_hex ? parseInt(m_hex[1], 16) : m_dec ? parseInt(m_dec[1], 10) : KeylayoutToKmnConverter.MAX_CTRL_CHARACTER : KeylayoutToKmnConverter.MAX_CTRL_CHARACTER
const ctr_val = ((m_uni || m_hex || m_dec)
? m_uni
? parseInt(m_uni[1], 16)
: m_hex
? parseInt(m_hex[1], 16)
: m_dec
? parseInt(m_dec[1], 10)
: KeylayoutToKmnConverter.MAX_CTRL_CHARACTER
: KeylayoutToKmnConverter.MAX_CTRL_CHARACTER

and I think this can be simplified to:

Suggested change
const ctr_val = ((m_uni || m_hex || m_dec) ?
m_uni ? parseInt(m_uni[1], 16) : m_hex ? parseInt(m_hex[1], 16) : parseInt(m_dec[1], 10) : KeylayoutToKmnConverter.MAX_CTRL_CHARACTER
m_uni ? parseInt(m_uni[1], 16) : m_hex ? parseInt(m_hex[1], 16) : m_dec ? parseInt(m_dec[1], 10) : KeylayoutToKmnConverter.MAX_CTRL_CHARACTER : KeylayoutToKmnConverter.MAX_CTRL_CHARACTER
const ctr_val = (
m_uni
? parseInt(m_uni[1], 16)
: m_hex
? parseInt(m_hex[1], 16)
: m_dec
? parseInt(m_dec[1], 10)
: KeylayoutToKmnConverter.MAX_CTRL_CHARACTER

);

// for control characters in 'U+...', '&#x...' or '&#...' format as well as in "" format
Expand All @@ -1670,7 +1680,7 @@ export class KmnFileWriter {
}
}
else {
out.character = this.convertToUnicodeCharacter(ctr);;
out.character = this.convertToUnicodeCharacter(ctr) ?? "";
}
return out;
}
Expand All @@ -1681,7 +1691,7 @@ export class KmnFileWriter {
* @param inputString the value that will converted
* @return a unicode character like 'c', 'ሴ', '😎' or undefined if inputString is not recognized
*/
public convertToUnicodeCharacter(inputString: string): string {
public convertToUnicodeCharacter(inputString: string): string | undefined {


// null, undefined will later be refused for conversion
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE keyboard SYSTEM "../../../../../resources/standards-data/Keylayout/Keylayout.dtd">
<!--

Data generated August 1st 2025

Generated by S. Schmitt
more keyMapSelect than KeyMap

-->
<keyboard group="126" id="-1272" name="Test_DifferentAmountOfMapSelectInKeyMapERROR" maxout="1">
<layouts>
<layout first="0" last="0" mapSet="138" modifiers="30"/>
</layouts>
<!-- different amount of tags MapSelect(2) vs. KeyMap(3) -->
<modifierMap id="30" defaultIndex="0">
<keyMapSelect mapIndex="0">
<modifier keys=""/>
<modifier keys="shift? caps? "/>
</keyMapSelect>
<keyMapSelect mapIndex="1">
<modifier keys="caps"/>
</keyMapSelect>
</modifierMap>

<!-- different amount of tags MapSelect(2) vs. KeyMap(3) -->
<keyMapSet id="ANSI">
<keyMap index="0">
<key code="0" action="A_9"/>
<key code="1" output="s"/>
<key code="2" output="d"/>
<key code="3" output="f"/>
<key code="4" output="h"/>
<key code="9" output="v"/>
<key code="10" output="\"/>
</keyMap>
<keyMap index="1">
<key code="0" action="A_1"/>
<key code="1" output="S"/>
<key code="2" output="D"/>
<key code="3" output="F"/>
<key code="4" output="@"/>
<key code="5" output="G"/>
<key code="6" output="Z"/>
<key code="7" output="X"/>
<key code="8" output="C"/>
<key code="9" output="V"/>
<key code="10" output="\"/>
</keyMap>
</keyMapSet>
<keyMapSet id="JIS">
<keyMap index="0">
<key code="0" action="A_9"/>
<key code="1" output="s"/>
<key code="2" output="d"/>
<key code="3" output="f"/>
<key code="4" output="h"/>
<key code="9" output="v"/>
<key code="10" output="\"/>
</keyMap>
<keyMap index="1">
<key code="0" action="A_1"/>
<key code="1" output="ሴ"/>
<key code="2" output="😎"/>
</keyMap>
</keyMapSet>
<actions>
<action id="A_1">
<when state="none" output="A"/>
</action>
<action id="A_9">
<when state="none" output="A"/>
</action>
</actions>
<terminators>
<when state="1" output="ˆ"/>
</terminators>
</keyboard>
Loading
Loading