Merge pull request #9374 from keymanapp/feat/developer/9119-marker-update-spec-epic-ldml

chore(developer): update markers per review 🙀

Author: srl295

common/web/types/src/ldml-keyboard/pattern-parser.ts

@@ -50,12 +50,11 @@ export class MarkerParser {
   /**
    * Marker sentinel as a string - U+FFFF
    */
-  public static readonly SENTINEL = String.fromCodePoint(constants.marker_sentinel);
-
+  public static readonly SENTINEL = String.fromCodePoint(constants.uc_sentinel);
   /**
-   * Matches all markers.
+   * Marker code as a string - U+0008
    */
-  public static readonly SENTINEL_ALL_MARKERS = this.SENTINEL + this.SENTINEL;
+  public static readonly MARKER_CODE = String.fromCodePoint(constants.marker_code);
 
   /** Minimum ID (trailing code unit) */
   public static readonly MIN_MARKER_INDEX = constants.marker_min_index;
@@ -88,15 +87,15 @@ export class MarkerParser {
     if (n < MarkerParser.MIN_MARKER_INDEX || n > MarkerParser.ANY_MARKER_INDEX) {
       throw RangeError(`Internal Error: marker index out of range ${n}`);
     }
-    return this.SENTINEL + String.fromCharCode(n);
+    return this.SENTINEL + this.MARKER_CODE + String.fromCharCode(n);
   }
 
   /** @returns all marker strings as sentinel values */
   public static toSentinelString(s: string, markers?: OrderedStringList) : string {
     if (!s) return s;
     return s.replaceAll(this.REFERENCE, (sub, arg) => {
       if (arg === MarkerParser.ANY_MARKER_ID) {
-        return MarkerParser.SENTINEL_ALL_MARKERS;
+        return MarkerParser.markerOutput(MarkerParser.ANY_MARKER_INDEX);
       }
       if (!markers) {
         throw RangeError(`Internal Error: Could not find marker \\m{${arg}} (no markers defined)`);

common/web/types/test/ldml-keyboard/test-pattern-parser.ts

@@ -1,6 +1,8 @@
 import 'mocha';
 import { assert } from 'chai';
 import { ElementParser, ElementSegment, ElementType, MarkerParser, OrderedStringList, VariableParser } from '../../src/ldml-keyboard/pattern-parser.js';
+import { constants } from '@keymanapp/ldml-keyboard-constants';
+import { KMXFile } from '../../src/kmx/kmx.js';
 
 describe('Test of Pattern Parsers', () => {
   describe('should test MarkerParser', () => {
@@ -52,8 +54,8 @@ describe('Test of Pattern Parsers', () => {
       }
     });
     it('should be able to emit sentinel values', () => {
-      assert.equal(MarkerParser.markerOutput(295), '\uFFFF\u0127', 'Wrong sentinel value emitted');
-      assert.equal(MarkerParser.markerOutput(MarkerParser.ANY_MARKER_INDEX), MarkerParser.SENTINEL_ALL_MARKERS, 'Wrong sentinel value emitted for ffff');
+      assert.equal(MarkerParser.markerOutput(295), '\uFFFF\u0008\u0127', 'Wrong sentinel value emitted');
+      assert.equal(MarkerParser.markerOutput(MarkerParser.ANY_MARKER_INDEX), '\uFFFF\u0008\uFFFE', 'Wrong sentinel value emitted for ffff');
       assert.throws(() => MarkerParser.markerOutput(0)); // below MIN
       assert.throws(() => MarkerParser.markerOutput(0x10000)); // above MAX
     });
@@ -87,7 +89,7 @@ describe('Test of Pattern Parsers', () => {
       );
       assert.equal(MarkerParser.toSentinelString(
         `Give me \\m{a} and \\m{c}, or \\m{.}.`, markers),
-        `Give me \uFFFF\u0001 and \uFFFF\u0003, or \uFFFF\uFFFF.`
+        `Give me \uFFFF\u0008\u0001 and \uFFFF\u0008\u0003, or \uFFFF\u0008\uFFFE.`
       );
       assert.throws(() =>
         MarkerParser.toSentinelString(
@@ -102,6 +104,10 @@ describe('Test of Pattern Parsers', () => {
         )
       );
     });
+    it('should match some marker constants', () => {
+      assert.equal(constants.uc_sentinel, KMXFile.UC_SENTINEL);
+      assert.equal(constants.marker_code, KMXFile.CODE_DEADKEY);
+    });
   });
   describe('should test VariableParser', () => {
     // same test as for markers

core/include/ldml/keyboardprocessor_ldml.h

@@ -93,11 +93,11 @@
 #define LDML_LENGTH_VARS_ITEM 0x10
 #define LDML_LENGTH_VKEY 0xC
 #define LDML_LENGTH_VKEY_ITEM 0x8
-#define LDML_MARKER_ANY_INDEX 0xFFFF
-#define LDML_MARKER_MAX_COUNT 0xFFFD
-#define LDML_MARKER_MAX_INDEX 0xFFFE
+#define LDML_MARKER_ANY_INDEX 0xFFFE
+#define LDML_MARKER_CODE 0x8
+#define LDML_MARKER_MAX_COUNT 0xFFFC
+#define LDML_MARKER_MAX_INDEX 0xFFFD
 #define LDML_MARKER_MIN_INDEX 0x1
-#define LDML_MARKER_SENTINEL 0xFFFF
 #define LDML_META_SETTINGS_FALLBACK_OMIT 0x1
 #define LDML_META_SETTINGS_TRANSFORMFAILURE_OMIT 0x2
 #define LDML_META_SETTINGS_TRANSFORMPARTIAL_HIDE 0x4
@@ -134,6 +134,7 @@
 #define LDML_TRAN_FLAGS_ERROR 0x1
 #define LDML_TRAN_GROUP_TYPE_REORDER 0x1
 #define LDML_TRAN_GROUP_TYPE_TRANSFORM 0x0
+#define LDML_UC_SENTINEL 0xFFFF
 #define LDML_VARS_ENTRY_TYPE_SET 0x1
 #define LDML_VARS_ENTRY_TYPE_STRING 0x0
 #define LDML_VARS_ENTRY_TYPE_UNICODESET 0x2

core/include/ldml/keyboardprocessor_ldml.ts

@@ -615,16 +615,18 @@ class Constants {
   }
 
   // ---- marker stuff ----
-  /** sentinel value indicating a marker follows */
-  readonly marker_sentinel = 0xFFFF;
+  /** == kmx_file.UC_SENTINEL, always followed by `marker_code`, marker index 0x0001-0xfffe */
+  readonly uc_sentinel         = 0xFFFF;
+  /** == kmx_file.CODE_DEADKEY */
+  readonly marker_code         = 0x0008;
   /** minimum usable marker index */
-  readonly marker_min_index = 0x0001;
+  readonly marker_min_index    = 0x0001;
   /** index value referring to the 'any' marker match */
-  readonly marker_any_index = 0xFFFF;
+  readonly marker_any_index    = 0xFFFE;
   /** maximum marker index prior to the 'any' value */
-  readonly marker_max_index = this.marker_any_index - 1;
+  readonly marker_max_index    = this.marker_any_index - 1;
   /** maximum count of markers (not including 'any') */
-  readonly marker_max_count = this.marker_max_index - this.marker_min_index;
+  readonly marker_max_count    = this.marker_max_index - this.marker_min_index;
 
 };
 

core/src/kmx/kmx_plus.h

@@ -310,6 +310,10 @@ static_assert(sizeof(struct COMP_KMXPLUS_TRAN_GROUP) == LDML_LENGTH_TRAN_GROUP,
 static_assert(sizeof(struct COMP_KMXPLUS_TRAN_TRANSFORM) == LDML_LENGTH_TRAN_TRANSFORM, "mismatched size of tran transform");
 static_assert(sizeof(struct COMP_KMXPLUS_TRAN_REORDER) == LDML_LENGTH_TRAN_REORDER, "mismatched size of tran reorder");
 
+// assert some parallel constants
+static_assert(LDML_UC_SENTINEL == UC_SENTINEL, "mismatch: LDML_UC_SENTINEL");
+static_assert(LDML_MARKER_CODE == CODE_DEADKEY, "mismatch: LDML_MARKER_CODE");
+static_assert(LDML_MARKER_ANY_INDEX < UC_SENTINEL, "expected LDML_MARKER_ANY_INDEX < UC_SENTINEL");
 
 /* ------------------------------------------------------------------
  * bksp section

core/src/ldml/C9134_ldml_markers.md

@@ -31,11 +31,11 @@ Markers can appear in both 'emitting' and 'matching-only' areas:
 
 ## Theory / Encoding
 
-- Keyman already uses U_SENTINEL `U+FFFF` (noncharacter)
-- The general proposal here is to use the sequence `U+FFFF U+XXXX` to represent marker #XXXX (starting with `U+0001`)
+- Keyman already uses `UC_SENTINEL` `U+FFFF` (noncharacter), with `CODE_DEADKEY` (0x0008)
+- The general proposal here is to use the sequence `U+FFFF U+0008 U+XXXX` to represent marker #XXXX (starting with `U+0001`)
 - `U+FFFF` cannot otherwise occur in text, so it is unique
-- `U+FFFF U+FFFF` to indicate 'any marker'  corresponds to `\m{.}`
-- This scheme allows for 65,534 (0xFFFE) unique markers, from `U+FFFF U+0001` through `U+FFFF U+FFFE`
+- `U+FFFF U+0008 U+FFFE` to indicate 'any marker'  corresponds to `\m{.}`
+- This scheme allows for 65,533 (0xFFFD) unique markers, from `U+FFFF U+0008 U+0001` through `U+FFFF U+0008 U+FFFD`
 
 ## Terminology
 - A marker's "number" is its position in the `markers` list, starting at index 1 (U+0001) being the first element in that list.
@@ -55,20 +55,20 @@ Note that this is different from other 0-based indices in KMX+. If there are thr
 ### Other sections
 
 - `string value='\m{…}'` will simply store `\m{…}`, for application when expanded as with other variables.
-- Other emitters, such as `key`, `transform` will include the string `U+FFFF U+XXXX` where XXXX corresponds to the marker's 1-based number.
-- Transforms will need to match against the marker or markers desired, so may need to emit sequences such as `(?:\uFFFF\u0123)` meaning a match to marker #0x0123
-- matching `\m{.}` may then expand to `(?:\uFFFF.)`
+- Other emitters, such as `key`, `transform` will include the string `U+FFFF U+0008 U+XXXX` where XXXX corresponds to the marker's 1-based number.
+- Transforms will need to match against the marker or markers desired, so may need to emit sequences such as `(?:\uFFFF\u0008\u0123)` meaning a match to marker #0x0123
+- matching `\m{.}` may then expand to `(?:\uFFFF\u0008.)` (match a single codepoint after `UC_SENTINEL CODE_DEADKEY`)
 
 ## Binary (.kmx plus)
 
 - The `vars.markers` is a pointer into the `list` section with a list (binary order) of the marker names
-- Other strings will be in `U+FFFF U+0123` form etc. as if it was in the original text stream as such.
+- Other strings will be in `U+FFFF U+0008 U+0123` form etc. as if it was in the original text stream as such.
 
 ## Implementation (core)
 
-- Core needs to recognize `U+FFFF …` sequences and convert them to markers in the context stream, with `state->context().push_marker(marker_number)`
-- For normal processing, Core does _not_ need to correlate the marker _number_ with a marker _id_, although this would be helpful for a debugging or tracing facility.  I.e. `U+FFFF U+0123` corresponding to entry 0x0123 in the `vars.markers` -> `list` table.
-- Core needs to remove `U+FFFF …` sequences before they are passed to the OS.
+- Core needs to recognize `U+FFFF U+0008 …` sequences and convert them to markers in the context stream, with `state->context().push_marker(marker_number)`
+- For normal processing, Core does _not_ need to correlate the marker _number_ with a marker _id_, although this would be helpful for a debugging or tracing facility.  I.e. `U+FFFF U+0008 U+0123` corresponding to entry 0x0123 in the `vars.markers` -> `list` table.
+- Core needs to remove `U+FFFF U+0008 …` sequences before they are passed to the OS.
 
 - Transform processing needs to recognize these markers in the context stream and pass them to the transforms appropriately.
 - User-defined backspace processing `<transform type="backspace"/> may specifically operate on backspaces in the context stream, just as with other transform processing.

developer/src/kmc-ldml/test/fixtures/basic.txt

@@ -465,7 +465,7 @@ block(strs)                         #  struct COMP_KMXPLUS_STRS {
   block(strKeys)             90 17 b6 17 block(x) 00 00                               # 'ថា'
   # <reorder before="ᩫ" from="᩠᩵ᩅ" order="10 55 10" />
   block(strIndicator)        3d d8 40 de block(x) 00 00                               # '🙀'
-  block(strSentinel0001)     FF FF 01 00 block(x) 00 00                   # U+FFFF U+0001
+  block(strSentinel0001)     FF FF 08 00 01 00 block(x) 00 00                   # UC_SENTINEL CODE_DEADKEY U+0001
 
 
 

developer/src/kmc-ldml/test/test-vars.ts

@@ -4,8 +4,10 @@ import { VarsCompiler } from '../src/compiler/vars.js';
 import { CompilerMessages } from '../src/compiler/messages.js';
 import { CompilerMessages as KmnCompilerMessages } from '@keymanapp/kmc-kmn';
 import { testCompilationCases } from './helpers/index.js';
-import { KMXPlus } from '@keymanapp/common-types';
+import { KMXPlus, KMX } from '@keymanapp/common-types';
 import { BASIC_DEPENDENCIES } from '../src/compiler/empty-compiler.js';
+import { constants } from '@keymanapp/ldml-keyboard-constants';
+
 // now that 'everything' depends on vars, we need an explicit dependency here
 const varsDependencies = BASIC_DEPENDENCIES.filter(c => c !== VarsCompiler);
 
@@ -186,6 +188,12 @@ describe('vars', function () {
       ],
     },
   ], varsDependencies);
+  describe('should match some marker constants', () => {
+    // neither of these live here, but, common/web/types does not import ldml-keyboard-constants otherwise.
+
+    assert.equal(constants.uc_sentinel, KMX.KMXFile.UC_SENTINEL);
+    assert.equal(constants.marker_code, KMX.KMXFile.CODE_DEADKEY);
+  });
   describe('markers', function () {
     this.slow(500); // 0.5 sec -- json schema validation takes a while