feat(input): configurable keymap (Phase 1) — data-driven shortcuts + macOS ⌘ defaults#29
feat(input): configurable keymap (Phase 1) — data-driven shortcuts + macOS ⌘ defaults#29MauricioAntunez wants to merge 8 commits into
Conversation
MauricioAntunez
force-pushed
the
feat/macos-cmd-keys
branch
2 times, most recently
from
7fe6d86 to
9b7353c
Compare
MauricioAntunez
changed the title
The input layer only read Control/Shift/Alt; the Super/Command modifier was never inspected, so on macOS the platform-standard ⌘ shortcuts did nothing — most notably ⌘V did not paste (only Ctrl+Shift+V worked). Route the Super modifier to a curated map mirroring mmterm's existing Ctrl/Ctrl+Shift bindings: ⌘V paste, ⌘C copy, ⌘N/⌘T new tab, ⌘W close tab, ⌘1–⌘9 select tab, ⌘Q quit, ⌘, config, ⌘F search, ⌘K clear scrollback, and ⌘+/⌘-/⌘= font size (⌘=/⌘0 reset). While Super is held an unmapped key is swallowed so a bare ⌘<key> never leaks to the PTY. handle_key is split into a thin winit adapter plus a pure, unit-testable handle_key_modified(key, ctrl, shift, alt, cmd, ...) so the modifier dispatch is covered without constructing winit KeyEvent/Modifiers. handle_key_inner and its 157 existing test call sites are untouched. Cross-platform (no cfg): Super-combos collide with no existing mmterm binding and keep the routing unit-testable on the Linux CI runner.
…(Phase 1 Tasks 1-3)
…ation (Phase 1 Task 4)
…nding notice (Phase 1 Tasks 5-7)
…Map/checklist, CHANGELOG (Phase 1 Task 8)
…+ config round-trip guards
MauricioAntunez
force-pushed
the
feat/macos-cmd-keys
branch
from
9e62d46 to
1b1b214
Compare
|
|
||
| // Chord: split on the first ASCII space into head + tail. | ||
| let mut parts = rest.splitn(2, ' '); | ||
| let head = parts.next().unwrap_or("").trim(); |
There was a problem hiding this comment.
the scopes normal and visual are accepted and pass the validation without error in from_config the guard ShadowsInput ( ~440) only reject the bindings in ModeClass::Global, silently accepted, stored, and never dispatched.
If there a binding like "normal:g" = "scroll_to_top" is stored by never executed. There maybe should show to the user a warning about that
| } | ||
|
|
||
| #[test] | ||
| fn ctrl_alt_b_also_toggles_passthrough() { |
There was a problem hiding this comment.
This test documented that Ctrl+Alt+B toggled passthrough (Ctrl taking priority over Alt for char keys). With the new keymap, Mods { ctrl:true, alt:true, key:"b" } does not match the M_CTRL + "b" entry (which has alt:false), so it falls through to the encoder and sends raw byte 0x02 to the PTY instead.
This is a silent behavior change. Either add a ctrl+alt+b row to default_keymap() pointing to toggle_passthrough`, or add a regression test that documents the new behavior explicitly.
| } | ||
|
|
||
| pub fn lookup(&self, scope: ModeClass, key: &BindingKey) -> Option<&'static str> { | ||
| self.map.get(&(scope, key.clone())).copied() |
There was a problem hiding this comment.
key.clone() allocates on every keypress: KeyToken::Char owns a String, and chord_tail carries a second one. HashMap supports borrow-based lookup via the
Borrow trait, which would avoid both allocations on the hot path. At minimum, a // TODO: avoid clone via Borrow comment would flag this for a future pass.
| /// Intern a validated action name to `&'static str`. Covers every name accepted | ||
| /// by `action_from_name`, including the parameterized ones that `name_of_action` | ||
| /// cannot reverse. Keep in sync with `action_from_name`. | ||
| fn intern_known_name(name: &str) -> Option<&'static str> { |
There was a problem hiding this comment.
every new action required keep sincronized 3 parts on this
1. `action_from_name` (~ 265)
2. `intern_known_name::NAMES` (~ 479)
3. `name_of_action` (#[cfg(test)], ~ 340)
Maybe a solution is define one place as table and used it as mapping.
roramirez
left a comment
roramirez
left a comment
There was a problem hiding this comment.
Thanks for your PR but i left some comments is required addressing before to merge the PR.
2 participants
Summary
Turns mmterm's modifier-shortcut keybindings into a single, data-driven, user-overridable keymap, and ships the macOS Command (⌘) / Linux Super shortcuts as part of it.
Previously every binding was hardcoded and ⌘V did not paste on macOS. Now
default_keymap()defines all built-in Global-scope shortcuts (Ctrl / Ctrl+Shift / Alt / ⌘-Super, andCtrl+Wpane chords) as data, and a new[keybindings]config table overlays them.What you can do
ctrl shift alt cmd(order-insensitive);cmd= ⌘ on macOS, Super on Linux/Windows."none"disables a default (returns the key to its raw terminal meaning).Architecture
Single source of truth in
src/input/keymap.rs. Dispatch consults the keymap first; literal text / cursor / PTY-byte encoding stays as the untouched fallback ("bindings are data; typing is encoded").KeyMap::from_configmerges defaults + user config and collects per-entry errors.Validation
Invalid entries (unparseable, unknown action, or a bare unmodified-character binding in global scope that would shadow typing) are skipped, logged, and surfaced as a transient status-bar notice — the app always starts.
Tests
New
src/input/keymap_test.rs(parser, default+overlay merge, "none" removal, lookup, validation) plus dispatch integration tests covering Ctrl+⌘ both-held, ⌘+key in Insert vs Normal, unmapped→fallback, override-replaces-default, "none"→raw, chord override (incl. case-insensitive tails), and Alt+Tab/Ctrl+C-in-Visual preservation. cargo fmt / clippy -D warnings / cargo test green (excluding pre-existing /bin/true PTY env failures on macOS).Docs
README / doc/SPEC.md (Configurable Keymap section + action-name table) and doc/LLMs.md (File Map, 4-step checklist, implemented section) updated.
Scope
Phase 1 = modifier shortcuts + chords + ⌘ defaults. Phase 2 (modal
normal:/visual:remapping) is future work.