Skip to content

Document HID modifier codes (0xE0..0xE7) with hex and constants - #711

Closed
rgoulter wants to merge 2 commits into
masterfrom
fix/modifier-hid-magic-numbers
Closed

Document HID modifier codes (0xE0..0xE7) with hex and constants#711
rgoulter wants to merge 2 commits into
masterfrom
fix/modifier-hid-magic-numbers

Conversation

@rgoulter

Copy link
Copy Markdown
Owner

Replaces decimal 224..231 magic numbers with self-documenting forms.

Nickel (45a75aa): key-extensions and tap_hold codegen now use hid.LeftCtrl etc via record lookup with 0xE0..0xE7; checks use 0xE0 LeftCtrl hex where const not available.

Rust (166c957): adds HID_LEFT_CTRL..HID_RIGHT_GUI constants and updates from_key_code, is_modifier_key_code, as_key_codes, as_byte and tests; no redundant comments after HID const.

Replace decimal 224..231 magic numbers with self-documenting forms:

- key-extensions.ncl: replace 224 <= kc <= 231 range test and
  224..231 match with a let hid = import "hid-usage-keyboard.ncl"
  and modifier_by_key_code record keyed by stringified hid.LeftCtrl
  etc, with # 0xE0..0xE7 comments. Lookup via has_field/get instead
  of pattern matching on numbers.

- tap_hold/keymap-codegen.ncl: replace 224..231 => 1..128 match for
  kc_mod with key_code_to_mod_byte record built from hid.LeftCtrl
  etc. Hex mapping documented per arm (0xE0 -> 0x01, etc).

- checks: use 0xE0 # LeftCtrl etc instead of 224 decimal where a
  const cannot be used; hex is more readable and comment clarifies
  intent. Expected rust_expr strings remain decimal (std.to_string).

No behavioural change; nickel eval checks still pass.
Replace raw 0xE0..0xE7 magic numbers with named constants that are
self-documenting; add comments only where raw hex remains without a
const.

- key::KeyboardModifiers: add HID_LEFT_CTRL..HID_RIGHT_GUI = 0xE0..0xE7
  with doc comments (HID usage for LeftCtrl (0xE0), etc). Update
  from_key_code to match on Self::HID_* instead of 0xE0 literals,
  is_modifier_key_code to use HID_LEFT_CTRL..=HID_RIGHT_GUI range,
  as_key_codes to push HID_* constants, as_byte to subtract
  HID_LEFT_CTRL. Constant definition is the single source of truth.

- caps_word.rs, tri_state.rs, key_lock.rs, keymap.rs,
  keymap_full_system.rs: replace 0xE0/0xE1/0xE2 literals with
  KeyboardModifiers::HID_* in tests and caps_word logic. Since the
  const already documents LeftCtrl etc, redundant // 0xE0 LeftCtrl
  comments are omitted; hex/comment kept only at const definitions.

No behavioural change; 0xE0 hex retained at definitions for
readability, const name carries intent elsewhere.
Comment on lines +4 to -24
let hid = import "hid-usage-keyboard.ncl" in
let modifier_by_key_code = {
"%{std.to_string hid.LeftCtrl}" = { left_ctrl = true }, # 0xE0 LeftCtrl -> left_ctrl
"%{std.to_string hid.LeftShift}" = { left_shift = true }, # 0xE1 LeftShift -> left_shift
"%{std.to_string hid.LeftAlt}" = { left_alt = true }, # 0xE2 LeftAlt -> left_alt
"%{std.to_string hid.LeftGUI}" = { left_gui = true }, # 0xE3 LeftGUI -> left_gui
"%{std.to_string hid.RightCtrl}" = { right_ctrl = true }, # 0xE4 RightCtrl -> right_ctrl
"%{std.to_string hid.RightShift}" = { right_shift = true }, # 0xE5 RightShift -> right_shift
"%{std.to_string hid.RightAlt}" = { right_alt = true }, # 0xE6 RightAlt -> right_alt
"%{std.to_string hid.RightGUI}" = { right_gui = true }, # 0xE7 RightGUI -> right_gui
}
in
hid
|> std.record.map_values (fun kc =>
kc
|> match {
# 0xE0 .. 0xE7
_ if 224 <= kc && kc <= 231 =>
{
modifiers =
kc
|> match {
224 => { left_ctrl = true },
225 => { left_shift = true },
226 => { left_alt = true },
227 => { left_gui = true },
228 => { right_ctrl = true },
229 => { right_shift = true },
230 => { right_alt = true },
231 => { right_gui = true },
},
},
kc => { key_code = kc },

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This is a bad change. :|

@rgoulter rgoulter closed this Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant