Merge pull request #11118 from keymanapp/feat/core/11072-default-modi…

…fier feat(core): support modifiers=other🙀
keymanapp · Apr 2, 2024 · 9333dfa · 9333dfa
2 parents 4610d1b + 5b1c853
commit 9333dfa
Show file tree

Hide file tree

Showing 12 changed files with 84 additions and 16 deletions.
diff --git a/common/include/kmx_file.h b/common/include/kmx_file.h
@@ -302,6 +302,9 @@ namespace kmx {
 
 #define K_MODIFIERFLAG  0x007F
 #define K_NOTMODIFIERFLAG 0xFF00   // I4548
+// Note: OTHER_MODIFIER = 0x10000, used by KMX+ for the
+// other modifier flag in layers, > 16 bit so not available here.
+// See keys_mod_other in keyman_core_ldml.ts
 
 struct COMP_STORE {
   KMX_DWORD_unaligned dwSystemID;

diff --git a/common/web/keyboard-processor/src/text/codes.ts b/common/web/keyboard-processor/src/text/codes.ts
@@ -1,7 +1,7 @@
 // TODO:  Move to separate folder:  'codes'
 // We should start splitting off code needed by keyboards even without a KeyboardProcessor active.
-// There's an upcoming `/common/web/types` package that 'codes' and 'keyboards' may fit well within.
-// In fact, there's a file there (on its branch) that should be merged with this one!
+
+// see also: common/web/types/src/kmx/kmx.ts
 
 const Codes = {
   // Define Keyman Developer modifier bit-flags (exposed for use by other modules)
@@ -25,6 +25,10 @@ const Codes = {
     "NO_SCROLL_LOCK":0x2000,  // NOTSCROLLFLAG
     "VIRTUAL_KEY":0x4000,     // ISVIRTUALKEY
     "VIRTUAL_CHAR_KEY":0x8000 // VIRTUALCHARKEY // Unused by KMW, but reserved for use by other Keyman engines.
+
+    // Note: keys_mod_other = 0x10000, used by KMX+ for the
+    // other modifier flag in layers, > 16 bit so not available here.
+    // See keys_mod_other in keyman_core_ldml.ts
   },
 
   modifierBitmasks: {
@@ -168,4 +172,4 @@ const Codes = {
   }
 }
 
-export default Codes;
+export default Codes;
diff --git a/common/web/types/src/kmx/kmx.ts b/common/web/types/src/kmx/kmx.ts
@@ -345,6 +345,10 @@ export class KMXFile {
   public static readonly ISVIRTUALKEY   = 0x4000;    // It is a Virtual Key Sequence
   public static readonly VIRTUALCHARKEY = 0x8000;    // Keyman 6.0: Virtual Key Cap Sequence NOT YET
 
+  // Note: OTHER_MODIFIER = 0x10000, used by KMX+ for the
+  // other modifier flag in layers, > 16 bit so not available here.
+  // See keys_mod_other in keyman_core_ldml.ts
+
   public static readonly MASK_MODIFIER_CHIRAL = KMXFile.LCTRLFLAG | KMXFile.RCTRLFLAG | KMXFile.LALTFLAG | KMXFile.RALTFLAG;
   public static readonly MASK_MODIFIER_SHIFT = KMXFile.K_SHIFTFLAG;
   public static readonly MASK_MODIFIER_NONCHIRAL = KMXFile.K_CTRLFLAG | KMXFile.K_ALTFLAG;
@@ -457,4 +461,4 @@ export class KMXFile {
       throw "COMP_KEYBOARD size is "+this.COMP_KEYBOARD.size()+" but should be "+KMXFile.COMP_KEYBOARD_SIZE+" bytes";
     }
   }
-}
+}
diff --git a/common/windows/cpp/include/legacy_kmx_file.h b/common/windows/cpp/include/legacy_kmx_file.h
@@ -320,6 +320,10 @@
 #define K_MODIFIERFLAG  0x007F
 #define K_NOTMODIFIERFLAG 0xFF00   // I4548
 
+// Note: OTHER_MODIFIER = 0x10000, used by KMX+ for the
+// other modifier flag in layers, > 16 bit so not available here.
+// See keys_mod_other in keyman_core_ldml.ts
+
 /*
   These sanity checks help ensure we don't
   break on-disk struct sizes when we cross

diff --git a/common/windows/delphi/keyboards/kmxfileconsts.pas b/common/windows/delphi/keyboards/kmxfileconsts.pas
@@ -129,6 +129,10 @@ interface
   KMX_ISVIRTUALKEY   = $4000;	// It is a Virtual Key Sequence
   KMX_VIRTUALCHARKEY = $8000; // It is a virtual character key sequence - mnemonic layouts
 
+  // Note: KMX_OTHER_MODIFIER = $10000, used by KMX+ for the
+  // other modifier flag in layers, > 16 bit so not available here.
+  // See keys_mod_other in keyman_core_ldml.ts
+
   // Combinations of key masks
   KMX_MASK_MODIFIER_CHIRAL = KMX_LCTRLFLAG or KMX_RCTRLFLAG or KMX_LALTFLAG or KMX_RALTFLAG;
   KMX_MASK_MODIFIER_SHIFT = KMX_SHIFTFLAG;

diff --git a/core/include/ldml/keyman_core_ldml.h b/core/include/ldml/keyman_core_ldml.h
@@ -34,7 +34,7 @@
 #define LDML_FINL_FLAGS_ERROR 0x1
 #define LDML_KEYS_KEY_FLAGS_EXTEND 0x1
 #define LDML_KEYS_KEY_FLAGS_GAP 0x2
-#define LDML_KEYS_MOD_ALL 0x17F
+#define LDML_KEYS_MOD_ALL 0x1017F
 #define LDML_KEYS_MOD_ALT 0x40
 #define LDML_KEYS_MOD_ALTL 0x4
 #define LDML_KEYS_MOD_ALTR 0x8
@@ -43,6 +43,7 @@
 #define LDML_KEYS_MOD_CTRLL 0x1
 #define LDML_KEYS_MOD_CTRLR 0x2
 #define LDML_KEYS_MOD_NONE 0x0
+#define LDML_KEYS_MOD_OTHER 0x10000
 #define LDML_KEYS_MOD_SHIFT 0x10
 #define LDML_LAYR_LIST_HARDWARE_TOUCH "touch"
 #define LDML_LENGTH_BKSP 0xC

diff --git a/core/include/ldml/keyman_core_ldml.ts b/core/include/ldml/keyman_core_ldml.ts
@@ -270,6 +270,11 @@ class Constants {
    */
   readonly keys_mod_shift = 0x0010;
 
+  /**
+   * bitmask for 'other'.
+   */
+  readonly keys_mod_other = 0x10000;
+
   /**
    * Convenience map for modifiers
    */
@@ -284,6 +289,7 @@ class Constants {
       ["ctrlL", this.keys_mod_ctrlL],
       ["ctrlR", this.keys_mod_ctrlR],
       ["shift", this.keys_mod_shift],
+      ["other", this.keys_mod_other],
     ]
   );
 

diff --git a/core/src/kmx/kmx_plus.cpp b/core/src/kmx/kmx_plus.cpp
@@ -42,6 +42,7 @@ static_assert(LALTFLAG == LDML_KEYS_MOD_ALTL, "LDML modifier bitfield vs. kmx_fi
 static_assert(K_ALTFLAG == LDML_KEYS_MOD_ALT, "LDML modifier bitfield vs. kmx_file.h #define mismatch");
 static_assert(CAPITALFLAG == LDML_KEYS_MOD_CAPS, "LDML modifier bitfield vs. kmx_file.h #define mismatch");
 static_assert(K_SHIFTFLAG == LDML_KEYS_MOD_SHIFT, "LDML modifier bitfield vs. kmx_file.h #define mismatch"); // "either" shift
+// LDML_KEYS_MOD_OTHER is not present in kmx_file.h (>16 bit)
 
 /**
  * \def LDML_IS_VALID_MODIFIER_BITS test whether x is a valid modifier bitfield

diff --git a/core/src/ldml/C7043_ldml.md b/core/src/ldml/C7043_ldml.md
@@ -461,17 +461,18 @@ For each key:
   by the compiler.
 - `mod`: 32-bit bitfield defined as below. Little endian values.
 
-|  Value   | Meaning  |`kmx_file.h`   | Comment                                     |
-|----------|----------|---------------|---------------------------------------------|
-|  0x0000  | `none`   |               | All zeros = no modifiers                    |
-|  0x0001  | `ctrlL`  | `LCTRLFLAG`   | Left Control                                |
-|  0x0002  | `ctrlR`  | `RCTRLFLAG`   | Right Control                               |
-|  0x0004  | `altL`   | `LALTFLAG`    | Left Alt                                    |
-|  0x0008  | `altR`   | `RALTFLAG`    | Right Alt                                   |
-|  0x0010  | `shift`  | `K_SHIFTFLAG` | Either Shift                                |
-|  0x0020  | `ctrl`   | `K_CTRLFLAG`  | Either Control                              |
-|  0x0040  | `alt`    | `K_ALTFLAG`   | Either Alt                                  |
-|  0x0100  | `caps`   | `CAPITALFLAG` | Caps lock                                   |
+|  Value   | Meaning   |`kmx_file.h`        | Comment                                       |
+|----------|-----------|--------------------|-----------------------------------------------|
+|  0x0000  | `none`    |                    | All zeros = no modifiers                      |
+|  0x0001  | `ctrlL`   | `LCTRLFLAG`        | Left Control                                  |
+|  0x0002  | `ctrlR`   | `RCTRLFLAG`        | Right Control                                 |
+|  0x0004  | `altL`    | `LALTFLAG`         | Left Alt                                      |
+|  0x0008  | `altR`    | `RALTFLAG`         | Right Alt                                     |
+|  0x0010  | `shift`   | `K_SHIFTFLAG`      | Either Shift                                  |
+|  0x0020  | `ctrl`    | `K_CTRLFLAG`       | Either Control                                |
+|  0x0040  | `alt`     | `K_ALTFLAG`        | Either Alt                                    |
+|  0x0100  | `caps`    | `CAPITALFLAG`      | Caps lock                                     |
+|  0x10000 | `other` | n/a                  | Other (not used in conjunction with others)   |
 
 TODO-LDML: Note that conforming to other keyman values, left versus right shift
 cannot be distinguished.

diff --git a/core/src/ldml/ldml_vkeys.cpp b/core/src/ldml/ldml_vkeys.cpp
@@ -7,6 +7,7 @@
 
 #include "ldml_vkeys.hpp"
 #include "kmx_file.h"
+#include <ldml/keyman_core_ldml.h>
 
 namespace km {
 namespace core {
@@ -65,6 +66,16 @@ vkeys::lookup(km_core_virtual_key vk, uint16_t modifier_state, bool &found) cons
       return ret;
     }
   }
+
+  // look for a layer with "other"
+  {
+    const vkey_id id_default(vk, (LDML_KEYS_MOD_OTHER));
+    ret = lookup(id_default, found);
+    if (found) {
+      return ret;
+    }
+  }
+
   // default: return failure. found=false.
   return ret;
 }

diff --git a/core/tests/unit/ldml/keyboards/k_012_other.xml b/core/tests/unit/ldml/keyboards/k_012_other.xml
@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+
+@@keys: [LALT K_BKQUOTE]
+@@expected: \u0064
+
+will match the default layer
+
+-->
+<keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
+  <info author="srl295" indicator="🙀" layout="qwerty"  name="TestKbd"/>
+
+  <keys/>
+
+  <layers formId="us">
+    <layer id="base" modifiers="none">
+      <row keys="b" /> <!-- number row -->
+    </layer>
+    <layer id="shift" modifiers="shift">
+      <row keys="s" /> <!-- number row -->
+    </layer>
+    <layer id="other" modifiers="other">
+      <row keys="d" /> <!-- number row -->
+    </layer>
+  </layers>
+</keyboard3>
diff --git a/core/tests/unit/ldml/keyboards/meson.build b/core/tests/unit/ldml/keyboards/meson.build
@@ -16,6 +16,7 @@ tests_from_cldr = [
   'bn',
 ]
 
+# these have 'embedded' (@@) testdata instead of a separate file
 tests_without_testdata = [
 # disabling 000 until we have updates to core or to the keyboard so that it passes
 #  'k_000_null_keyboard',
@@ -25,6 +26,7 @@ tests_without_testdata = [
   'k_005_modbittest',
   'k_010_mt',
   'k_011_mt_iso',
+  'k_012_other',
   'k_100_keytest',
   'k_101_keytest',
   'k_102_keytest',