feat: Allow for user-defined "chord" / multi-key keybindings (and correct vi motions when targeting <space> char) - #1016
Conversation
|
Am experimenting with the nushell-side of things for configuring in Currently, keychord configuration looks like this: $env.config.keybindings ++= [
{
name: "jj_normal"
modifier: "none"
keycode: [ "char_j", "char_j" ]
mode: "vi_insert"
event: { send: "vichangemode", mode: "normal" }
}
{
name: "save_buffer"
modifier: "control"
keycode: [ "char_x", "char_s" ]
mode: "emacs"
event: { send: "submit" }
}
{
name: "mixed_mods"
modifier: "none"
keycode: [
{ modifier: "alt", keycode: "char_w" }
{ modifier: "none", keycode: "char_j" }
]
mode: "emacs"
event: { send: "openeditor" }
}
]How do we feel about it? |
vi motions when targeting <space> char)vi motions when targeting <space> char)
|
I'm not in a position to review this, but just want to say that this is what's keeping me from using nushell – I've have a serious Edit. Actually I just quit my jk addiction and use caps lock as esc. Have been a happy nushell user for a few months now. |
|
Well this is a massive Change, I just overflew it. I understand that this is important to you and I am already thinking about a way how to make repeated/pending keys part of the editor such that Thanks, but as long as I can't make sure |
|
Very fair feedback. I've been dogfooding my fork of nushell using this branch for ~4-5 months now. If you feel this approach is valuable for the project, I'd be more than happy to revive this pr & ensure it's well-tested (incl. using your new Vi harness). Otherwise, I'm perfectly happy maintaining my personal fork. Totally your call! |
|
@benvansleen My idea: It might be cleanest to close this in favor of a fresh PR for the refactor, happy to look at it as a draft. |
|
Heads up Could the resolver sit on top of that trie rather than the flat list? Worth coordinating with @reubeno? |
|
Would be happy to! Think a trie is a great idea. Should have time to take a closer look at #989 this weekend. |
|
That sounds great, thanks @benvansleen! |
Replace the keybindings-crate module (dep dropped) with a hand-rolled dispatcher: PromptEditMode::Helix payload (rest policy + Span extent), count/pending sequence state, pure interpret/complete_pending/lower split, dispatch_insert (Esc before table, insert-self, bare Enter guard), and the EditCommand::Select / CollapseSelection core verbs. Port resolve_selection + word_boundary_at_origin from helix-mode-wip: helix range semantics for Select, incl. the boundary hop that makes repeated w tile words instead of sticking on spaces; Unicode word kind deliberately takes the default anchor rule. The Select Span arm goes through resolve_selection. Tests: machine table tests (interpret next_mode per operator x mode, counts, pending, Esc rules, insert path), resolve_selection specs, editor-level tiling + collapse coverage. Still open: feel-test b/e in the example, x / line-edge selection, j/k history question, Alt-d, g-menu waits on nushell#1016. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the keybindings-crate module (dep dropped) with a hand-rolled dispatcher: PromptEditMode::Helix payload (rest policy + Span extent), count/pending sequence state, pure interpret/complete_pending/lower split, dispatch_insert (Esc before table, insert-self, bare Enter guard), and the EditCommand::Select / CollapseSelection core verbs. Port resolve_selection + word_boundary_at_origin from helix-mode-wip: helix range semantics for Select, incl. the boundary hop that makes repeated w tile words instead of sticking on spaces; Unicode word kind deliberately takes the default anchor rule. The Select Span arm goes through resolve_selection. Tests: machine table tests (interpret next_mode per operator x mode, counts, pending, Esc rules, insert path), resolve_selection specs, editor-level tiling + collapse coverage. Still open: feel-test b/e in the example, x / line-edge selection, j/k history question, Alt-d, g-menu waits on nushell#1016.
Replace the keybindings-crate module (dep dropped) with a hand-rolled dispatcher: PromptEditMode::Helix payload (rest policy + Span extent), count/pending sequence state, pure interpret/complete_pending/lower split, dispatch_insert (Esc before table, insert-self, bare Enter guard), and the EditCommand::Select / CollapseSelection core verbs. Port resolve_selection + word_boundary_at_origin from helix-mode-wip: helix range semantics for Select, incl. the boundary hop that makes repeated w tile words instead of sticking on spaces; Unicode word kind deliberately takes the default anchor rule. The Select Span arm goes through resolve_selection. Tests: machine table tests (interpret next_mode per operator x mode, counts, pending, Esc rules, insert path), resolve_selection specs, editor-level tiling + collapse coverage. Still open: feel-test b/e in the example, x / line-edge selection, j/k history question, Alt-d, g-menu waits on nushell#1016.
Replace the keybindings-crate module (dep dropped) with a hand-rolled dispatcher: PromptEditMode::Helix payload (rest policy + Span extent), count/pending sequence state, pure interpret/complete_pending/lower split, dispatch_insert (Esc before table, insert-self, bare Enter guard), and the EditCommand::Select / CollapseSelection core verbs. Port resolve_selection + word_boundary_at_origin from helix-mode-wip: helix range semantics for Select, incl. the boundary hop that makes repeated w tile words instead of sticking on spaces; Unicode word kind deliberately takes the default anchor rule. The Select Span arm goes through resolve_selection. Tests: machine table tests (interpret next_mode per operator x mode, counts, pending, Esc rules, insert path), resolve_selection specs, editor-level tiling + collapse coverage. Still open: feel-test b/e in the example, x / line-edge selection, j/k history question, Alt-d, g-menu waits on nushell#1016.
Replace the keybindings-crate module (dep dropped) with a hand-rolled dispatcher: PromptEditMode::Helix payload (rest policy + Span extent), count/pending sequence state, pure interpret/complete_pending/lower split, dispatch_insert (Esc before table, insert-self, bare Enter guard), and the EditCommand::Select / CollapseSelection core verbs. Port resolve_selection + word_boundary_at_origin from helix-mode-wip: helix range semantics for Select, incl. the boundary hop that makes repeated w tile words instead of sticking on spaces; Unicode word kind deliberately takes the default anchor rule. The Select Span arm goes through resolve_selection. Tests: machine table tests (interpret next_mode per operator x mode, counts, pending, Esc rules, insert path), resolve_selection specs, editor-level tiling + collapse coverage. Still open: feel-test b/e in the example, x / line-edge selection, j/k history question, Alt-d, g-menu waits on nushell#1016.
Replace the keybindings-crate module (dep dropped) with a hand-rolled dispatcher: PromptEditMode::Helix payload (rest policy + Span extent), count/pending sequence state, pure interpret/complete_pending/lower split, dispatch_insert (Esc before table, insert-self, bare Enter guard), and the EditCommand::Select / CollapseSelection core verbs. Port resolve_selection + word_boundary_at_origin from helix-mode-wip: helix range semantics for Select, incl. the boundary hop that makes repeated w tile words instead of sticking on spaces; Unicode word kind deliberately takes the default anchor rule. The Select Span arm goes through resolve_selection. Tests: machine table tests (interpret next_mode per operator x mode, counts, pending, Esc rules, insert path), resolve_selection specs, editor-level tiling + collapse coverage. Still open: feel-test b/e in the example, x / line-edge selection, j/k history question, Alt-d, g-menu waits on nushell#1016.
Breaking: `PromptEditMode` gains a `Helix` variant, so helix stops reporting itself as `Vi` and exhaustive matches on that enum need a new arm. The `keybindings` crate is gone and `helix` is now a bare feature. Keeping `interpret`, `complete_pending` and `lower` free of `self` is what lets the mode table be tested without driving an editor, and it is the constraint the whole dispatcher is shaped around. `resolve_selection` and `word_boundary_at_origin` are ported from `helix-mode-wip`. Still open: feel-test `b`/`e` in the example, `x` and line-edge selection, the `j`/`k` history question, `Alt-d`, and the `g` menu, which waits on nushell#1016.
The prefix needs nothing from nushell#1016: that draft generalizes the binding *table*, and a fixed-key-set prefix like this is the only kind it could ever absorb. `f` and `r` take an arbitrary char as data, so `Pending` stays either way, and `g` costs nothing by joining it.
* fix(completion): complete at the far edge of the caret grapheme Every caret mode (vi normal, vi visual, helix normal and select) rests *on* a grapheme, and `insertion_point` reports that grapheme's start. Prefix completers read the position they are given as the *end* of the word, so completing at the caret stranded the grapheme it covered: "foo" + "foobar" gave "foobaro". * fix(hint): collapse the block caret before appending a history hint `set_insertion_point` moves only the head of an anchored cursor, so the append ran through `delete_selection` and ate the covered grapheme. Not reachable on this branch yet, since `is_cursor_at_buffer_end` still rejects any non-empty cursor. Carried anyway so that refining that guard cannot silently reintroduce the eating; on `helix-mode-wip` it is directly observable, where the hint "-add" on "ssh" lands as "ss-add". * feat(helix)!: rewrite helix mode as a pure state machine Breaking: `PromptEditMode` gains a `Helix` variant, so helix stops reporting itself as `Vi` and exhaustive matches on that enum need a new arm. The `keybindings` crate is gone and `helix` is now a bare feature. Keeping `interpret`, `complete_pending` and `lower` free of `self` is what lets the mode table be tested without driving an editor, and it is the constraint the whole dispatcher is shaped around. `resolve_selection` and `word_boundary_at_origin` are ported from `helix-mode-wip`. * feat(helix): bind undo and redo to `u` and `U` Both were unreachable: `interpret` rejected them and no helix table binds an undo chord. They live in the machine rather than the table, since `dispatch` consults the table only while no count is live, and a table-bound `u` would leave `3u` dead. Counts apply here unlike the other non-motion verbs, since stepping the edit stack back N times is what `3u` means. `next_mode` is `None` since select mode is sticky. * refactor(helix): name the count-repeat rule `Action::repeated` Six arms of `lower` spelled `vec![cmd; action.count]` inline, leaving the rule that governs them implicit. Naming it gives the deciding question a home and makes the exception visible at the call site: `p` will carry its count in the command instead, since repeating a paste re-anchors. * feat(helix): paste at the selection edge with `p` and `P` The three existing paste commands delete the selection first, which is right for vi but never a no-op in helix, where the resting cursor is always a min-width-1 selection: pasting `bar` onto `foo` gave `fobar`. `PasteAtSelectionEdge` is exempt from the `clear_selection` pass in `run_edit_command`, which assumes only selecting motions leave a selection behind. Safe to special-case since the command is new; exempting `EditType::UndoRedo` there would reach vi and emacs too. * feat(helix): bind the goto family to the `g` prefix The prefix needs nothing from #1016: that draft generalizes the binding *table*, and a fixed-key-set prefix like this is the only kind it could ever absorb. `f` and `r` take an arbitrary char as data, so `Pending` stays either way, and `g` costs nothing by joining it. * fix(editor): collapse the cursor before opening a line Only `o` was visibly wrong; `insert_newline_above` already landed correctly from an anchored cursor and changes here for the invariant, not for a bug. Nothing reached either path with a selection before: helix is the first mode whose resting cursor is always anchored. * feat(helix): open a line with `o` and `O` `O` would survive `Action::repeated`, since `find_char_left` still finds the previous newline from a blank line, but `o` will not, so both lower to one seeking open plus plain newlines rather than branching on the coincidence. * feat(helix): rest the cursor on the line terminator The resting selection now covers the `\n`, so every command that opens with `delete_selection` eats it: the counted `o` lowering had to stop using `InsertNewline` for exactly that reason, and any future helix binding lowering onto such a command needs the same care. The block is painted as a selection highlight, which a terminator has no width to show, so the caret falls back to the terminal's own shape there until the painter grows a cell for it. * fix(painter): give a selected line terminator a cell to paint Helix made it reachable by resting the cursor on the terminator, but the gap was never helix-only: a shift selection in emacs or vi that lands on a newline showed nothing either, and now shows the same cell. The cell is a real column, so `required_lines` and `cursor_pos` count it when they measure the rendered string. That only shows up on a line which exactly fills the terminal width. * refactor(helix)!: feature gate every helix-specific item Breaking under `--features helix`-off builds: `PromptHelixMode` and the `PromptEditMode::Helix` variant disappear, so exhaustive matches on that enum no longer need a helix arm. The variant, its `RestPolicy`, and the tests were all reachable before, which meant a custom `EditMode` could return `PromptEditMode::Helix` and get the full helix cursor semantics without the feature. One gate per region rather than per item, since all of it lifts in a single sweep once helix stops being gated: the editor tests move into a gated inner module, and the two `RestPolicy::Block` branches share a `block` helper so only the variant needs an attribute. * feat(prompt)!: give helix select its own discriminant Breaking: `PromptEditModeDiscriminants` gains `HelixSelect`, and helix select no longer reports as `HelixNormal`. A consumer naming modes by discriminant can now address select on its own; one keying off `HelixNormal` to reach it needs a second arm. The vi `Normal | Visual` pair stays bundled on purpose and is pinned by a test, so splitting it stays a deliberate separate change. * docs: cut comments that restate the code * fix(editor): depart a forward `Extend` from the head, not the caret Helix select mode's `l` and `w` never moved: under a block policy `insertion_point()` is the caret, a grapheme behind the head, so an exclusive forward motion resolved onto the boundary the previous `Extend` had already parked the head on. `motion_origin` reads that edge through `insertion_point()` rather than `Cursor::caret()`. Both are the caret under a block policy, but `caret()` is unconditional, thus a `Between` mode holding a forward selection would have started resolving a grapheme early. * refactor(core)!: stop `MotionTarget::Offset` reading as a displacement The other `offset` names in the tree stay. A motion target is the one place the relative reading is live, since motions are relative by nature; elsewhere `byte offset` already reads as an absolute position, thus renaming would be churn. * test(editor): drive helix select motions instead of asserting events The helix suite asserts the emitted `ReedlineEvent` and never runs a command, which is how the frozen `Extend` fixed in 227d974 survived. Most cases do not discriminate: revert 227d974 and only `w` and `t` fail. `e` and `f` are inclusive and `$`/`G` are absolute, thus they were never affected, and their passing is what pins the fix as narrow. * fix(core): stop an operator span slicing inside a grapheme `resolve_motion` looks like the place for this, since both the cursor and the operator path funnel through it, but it would have to floor uniformly and thus under-select a grapheme the caller pointed into. `operator_span` is the one that promises a sliceable range, and `recohere` already expands outward. Latent until something constructs a `Position`, so no existing span moves: `recohere` is idempotent and every span a motion produces is already aligned. * feat(helix): give each helix mode its own cursor shape The `hx_*` fields are gated, thus a literal that names every field of `CursorConfig` compiles under one feature set and not the other, and `..default()` is a no-op under the other. `examples/demo.rs` hit both in turn: it now leaves `emacs` to the update, which has an effect either way. The painter's `_ => None` arm meant helix silently got no cursor rather than a build error, unlike `rest_policy` and `selection_extent` which drop the catch-all on purpose so a new mode fails to compile. * fix(painter): paint the terminator whenever a selection covers it Both of `008df78`'s conditions were too narrow. It wanted a chunk that was *nothing but* a terminator, reachable only by stepping onto a `\n` from an empty line, and it wanted a background, while the examples style a selection with `Style::new().reverse()`, which leaves `background` at `None`. The cell thus never appeared in practice. Not helix-only: a vi visual selection crossing a line is the same shape. `split` also leaves the `\r` of a CRLF on the piece, painted raw, which sent the terminal to column 0 mid-line. And the caveat from `008df78` widens: any selected line ending in a terminator now gains a column, thus one that exactly fills the terminal wraps a row early. * fix(painter): pin the cursor when a line fills the terminal width A line filling the width exactly leaves the cursor in the deferred-wrap state: the glyph landed in the final column and the cursor is flagged pending rather than moved. DECSC records that flag, terminals disagree about whether DECRC carries it, and the cursor rendered either in the last cell of the row or the first of the next. Every edit mode was affected; a block cursor just makes it obvious. Each print path reports the row it left the cursor on rather than the caller recomputing it. A large buffer prints line-skipped text, so the rows on screen are not the rows the whole buffer needs, and judging the margin from the untrimmed text would be wrong there. * refactor(core): let `MotionTarget::direction` answer `None` A destination-shaped target names where to land, not which way to travel, thus its direction is only answerable once the head is resolved. Reading it off `origin` gave `Position` an answer that looked real. No behaviour change: `Position` resolves to the same head from any origin, thus the branch `motion_origin` picked for it never reached `resolve_motion`'s output. * feat(helix)!: goto the first non-blank with `gs` Breaking: `MotionTarget` gains `LineStartNonBlank` under the `helix` feature, thus an exhaustive match on it needs a new arm. Helix leaves the cursor alone on a whitespace-only line, thus `line::first_non_blank` bounds its search to the line and answers `None` there. `LineBuffer::line_non_blank_start_index` cannot serve: it searches on into the buffer and settles for the terminator, so it moves on a blank line, and that is existing vi behaviour worth leaving alone. `Extend` routes a destination-shaped target through `put_cursor` rather than `Span`'s `op_end`. Whether the landing grapheme is covered turns on which side of the anchor it falls, which `resolve_motion` cannot see, and upstream helix lowers `gs` through `put_cursor` in select mode too. * feat(helix): let a host supply helix keybindings `Vi::new` and `Emacs::new` take their tables, so helix was the only edit mode a host could not rebind at all, though `dispatch` already consults the normal table before reaching the state machine. Builders rather than a `new(insert, normal)`, since they compose with the `Default` plus `with_*` idiom the rest of the crate uses and let a caller replace one table while keeping the other's default. * feat(examples): run the demo in helix mode with `--helix` Helix testing wanted the history, completer and menus the demo already configures; `examples/helix.rs` stays minimal rather than growing a second copy of that setup. Two `#[cfg]`ed definitions of `helix_edit_mode` rather than a gate at the call site, which would have duplicated the vi and emacs arms across both configurations. That is what the vi and emacs extractions are for. The example carries no `required-features`, thus `--helix` is typeable without the feature compiled in and says so rather than falling through to emacs silently. * fix(helix): stop a submit from normal mode eating a grapheme The resting cursor is a selection and outlives the `next_mode` flip to insert, thus `InsertNewline` on incomplete input opened with `delete_selection` and ate the grapheme under the cursor, and `submit_buffer`'s extra repaint left the selection highlight in the scrollback. Collapsing forward rather than deselecting or releasing the way vi does: a helix head already sits on the far edge of the covered grapheme, so `Deselect` breaks before it and vi's `MoveRight` one beyond, which the end-of-buffer clamp hides until the cursor is mid-line. Only the incomplete branch can be asserted on, since a submitted buffer is cleared before anything can read it. * feat(helix): move by line or walk history with `j`/`k` Mirrors vi: an open menu takes the keys first, then line movement while another line is there, then history at the buffer edge, prefix-searched when the caret sits at the buffer end. Select mode extends by line and never reaches history, which would replace the buffer the selection is anchored in. * feat(helix)!: select whole lines with `x` Breaking: `EditCommand` gains `SelectLine` under the `helix` feature, thus `EditCommandDiscriminants` gains a variant. The command enum is `#[non_exhaustive]`; its generated discriminants are not. A command rather than a composition of existing ones, since only the "already spans whole lines" test can grow the selection on a repeat. `Select` re-anchors at the origin and `Extend` keeps its anchor, so neither moves both edges to line boundaries and then notices they were there already. * fix(helix): keep the selection standing after a yank or case change `run_edit_command` clears the selection for every command that is not a selecting motion, and the operations *on* a selection are all `EditType::NoOp` or `EditText`, so helix collapsed the very span they had just acted on. A mode policy rather than adding them to that allowlist: vi emits the same commands, and vim does drop the selection on `y` or `~`, thus a blanket exemption would strand a highlight in vi visual. The whole suite passes either way, which says the vi side is untested, not that it is free to change. * feat(helix)!: delete without yanking on `Alt-d` Breaking: `EditCommand` gains `EraseSelection` under the `helix` feature, thus `EditCommandDiscriminants` gains a variant. The command enum is `#[non_exhaustive]`; its generated discriminants are not. There was no way to drop text without clobbering the register, since `CutSelection` always fills it. `OperatorVerb::Erase` already performs the register-free deletion for the motion-shaped `Erase`, so only the span differs. The binding sits in the normal table, which select mode shares. * feat(helix): bind `%`, `A` and `I` All three lower onto commands that already exist, so this is bindings only. `A` reaches append position without help: `LineEdge(Forward)` lands before the terminator, and the switch to insert reads the head rather than the caret, which is one grapheme further along. `I` takes `LineStartNonBlank`, added for `gs`, rather than the line start, thus it lands on an indented line's first word. * feat(helix): switch the selection's case with `~` Bindings only: `SwitchcaseSelection` and the retention policy both predate this. Pressing it twice returns the original text, which is the observable form of the selection surviving the edit. * feat(helix): set the selection's case with `` ` `` and ``Alt-` `` Bindings only. They split across the two dispatch paths because of the modifier: the plain backtick is typeable so the state machine takes it, while `interpret` rejects Alt and the keybinding table picks that one up, as it already does for `Alt-d`. * docs: introduce the helix edit mode The feature was absent from the feature list and the mode from the edit-mode section, so nothing outside the source said it existed. The example is wrapped in a hidden `#[cfg(feature = "helix")]` block rather than marked `ignore`, thus it is type-checked under the feature and compiles away without it, instead of rotting unchecked in both. No keymap table here on purpose; that belongs with the reference already staged for landing. * docs: repair garbled doc comments around the helix additions * refactor(helix): compact the key tables and rename the char predicates * refactor(core): move the rest-policy predicates onto RestPolicy * fix(helix): cap the count prefix before it can freeze the REPL * refactor(edit_mode): dedup event handling and compact the helix tables * feat(helix): add the helix tables to the default-keybindings query * feat(helix): move the helix mode into the default feature set The nushell side wants the mode shipped enabled rather than opt-in. The feature flag stays, thus `default-features = false` builds still compile without it.
I care a great deal about nushell's vi mode support. I got myself addicted to "jj" to exit vi normal mode in basically all cli tools with a vi mode -- without it, I feel like I've lost my fingers!
A while back, I proposed #670. Since
reedlinedoes not support multi-key "chords," I special-cased logic intovi/mod.rsto listen for repeated keypresses in insert mode for a designated "exit-insert-mode" trigger.The feedback led to a broader discussion. The main gist (as I understood it at the time) was that we should not special-case this logic; ideally, we would generalize this functionality to enable other kinds of user-defined key chords.
In the meantime, I've been maintaining a personal
reedlinefork with this special-case logic. It's fulfilled my needs, but is kind of a PITA. So: I thought why not take another stab?Full disclosure: I've been trying to experiment with LSP-informed LLM code generation for languages with well-developed type systems (eg through something like rust LSP w/ opencode). The first draft of this PR was predominantly LLM generated, but I have reviewed the changes & am test-driving this as my daily-driver shell.
User-facing changes
ReedlineEvent:ViChangeModehandling to better mimic vi/vim behavior (eg when moving from insert -> normal modes, the cursor should move left 1 char)emacsandvi) now track a sequence state of keypressesf<space>does not behave as expected in nushell; it basically inserts a space at b.o.l. and jacks up the undo bufferWhat does this achieve for
nushell?in
nu-cli/src/reedline_config.rs, you could:Obviously, this would be better configured as part of the user's nushell startup script. If this PR is approved / we like this direction, I'll submit work on the nushell side to allow for configuring
$env.config.keybindingsaccordingly.How does it work?
Key event flow (Emacs / Vi):
KeyEvent -> normalize -> KeyCombination
-> KeySequenceState.process_combo(...)
-> SequenceResolution.into_event(|combo| fallback(combo))
-> ReedlineEvent returned to engine
Notes:
pending_exactholds an exact match that is also a prefix of a longer sequence.If the longer sequence fails, we emit the saved event and keep trailing keys.
SequenceResolution.events= matched sequencesSequenceResolution.combos= raw keys to replay through fallbackinto_eventcombines both into a single ReedlineEventVi specifics:
so
comboscan be re-fed into vi’s grammar instead of becoming edits.Timeout path:
Engine timeout -> EditMode.flush_pending_sequence()
-> KeySequenceState.flush_with_combos()
-> SequenceResolution.into_event(...)
-> ReedlineEvent
Walking through an example!
Scenario: insert mode, sequence binding
j j→ViChangeMode("normal".into())Initial state:
KeySequenceState.buffer = []pending_exact = Nonesequence_bindings contains [j, j] -> ViChangeMode("normal".into())Step 2: user presses j again
Result:
ViChangeMode("normal".into())is emitted immediately.Step 3: If the user doesn’t press another key
Failure/timeout example: