Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor key dispatch #14942

Merged
merged 4 commits into from
Jul 22, 2024
Merged

Refactor key dispatch #14942

merged 4 commits into from
Jul 22, 2024

Conversation

ConradIrwin
Copy link
Member

@ConradIrwin ConradIrwin commented Jul 22, 2024

Simplify key dispatch code.

Previously we would maintain a cache of key matchers for each context that
would store the pending input. For the last while we've also stored the
typed prefix on the window. This is redundant, we only need one copy, so now
it's just stored on the window, which lets us avoid the boilerplate of keeping
all the matchers in sync.

This stops us from losing multikey bindings when the context on a node changes
(#11009) (though we still interrupt multikey bindings if the focus changes).

While in the code, I fixed up a few other things with multi-key bindings that
were causing problems:

Previously we assumed that all multi-key bindings took precedence over any
single-key binding, now this is done such that if a user binds a single-key
binding, it will take precedence over all system-defined multi-key bindings
(irrespective of the depth in the context tree). This was a common cause of
confusion for new users trying to bind to cmd-k or ctrl-w in vim mode
(#13543).

Previously after a pending multi-key keystroke failed to match, we would drop
the prefix if it was an input event. Now we correctly replay it (#14725).

Release Notes:

  • Fixed multi-key shortcuts not working across completion menu changes (#11009)
  • Fixed multi-key shortcuts discarding earlier input (#14445)
  • vim: Fixed jk binding preventing you from repeating j (#14725)
  • vim: Fixed escape in normal mode to also clear the selected register. (#14311)
  • Fixed key maps so user-defined mappings take precedence over builtin multi-key mappings (#13543)
  • Fixed a bug where overridden shortcuts would still show in the Command Palette

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jul 22, 2024
@ConradIrwin ConradIrwin merged commit 2e23527 into main Jul 22, 2024
10 checks passed
@ConradIrwin ConradIrwin deleted the keymap-shred branch July 22, 2024 16:46
ConradIrwin added a commit that referenced this pull request Jul 24, 2024
Broken by #14942 as the matching Pane binding for toggle regex now takes
precednce. cmd-alt-x still works for that.
ConradIrwin added a commit that referenced this pull request Jul 24, 2024
Broken by #14942 as the matching Pane binding for toggle regex now takes
precedence. cmd-alt-x still works for that.

Release Notes:

- N/A
CharlesChen0823 pushed a commit to CharlesChen0823/zed that referenced this pull request Jul 29, 2024
Simplify key dispatch code.

Previously we would maintain a cache of key matchers for each context
that
would store the pending input. For the last while we've also stored the
typed prefix on the window. This is redundant, we only need one copy, so
now
it's just stored on the window, which lets us avoid the boilerplate of
keeping
all the matchers in sync.

This stops us from losing multikey bindings when the context on a node
changes
(zed-industries#11009) (though we still interrupt multikey bindings if the focus
changes).

While in the code, I fixed up a few other things with multi-key bindings
that
were causing problems:

Previously we assumed that all multi-key bindings took precedence over
any
single-key binding, now this is done such that if a user binds a
single-key
binding, it will take precedence over all system-defined multi-key
bindings
(irrespective of the depth in the context tree). This was a common cause
of
confusion for new users trying to bind to `cmd-k` or `ctrl-w` in vim
mode
(zed-industries#13543).

Previously after a pending multi-key keystroke failed to match, we would
drop
the prefix if it was an input event. Now we correctly replay it
(zed-industries#14725).

Release Notes:

- Fixed multi-key shortcuts not working across completion menu changes
([zed-industries#11009](zed-industries#11009))
- Fixed multi-key shortcuts discarding earlier input
([zed-industries#14445](zed-industries#14445))
- vim: Fixed `jk` binding preventing you from repeating `j`
([zed-industries#14725](zed-industries#14725))
- vim: Fixed `escape` in normal mode to also clear the selected
register.
- Fixed key maps so user-defined mappings take precedence over builtin
multi-key mappings
([zed-industries#13543](zed-industries#13543))
- Fixed a bug where overridden shortcuts would still show in the Command
Palette
CharlesChen0823 pushed a commit to CharlesChen0823/zed that referenced this pull request Jul 29, 2024
Broken by zed-industries#14942 as the matching Pane binding for toggle regex now takes
precedence. cmd-alt-x still works for that.

Release Notes:

- N/A
JosephTLyons pushed a commit that referenced this pull request Aug 2, 2024
Broken by #14942 as the matching Pane binding for toggle regex now takes
precedence. cmd-alt-x still works for that.

Release Notes:

- N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant