keymap_editor: Fix default binding deletion and override#50699
keymap_editor: Fix default binding deletion and override#50699probably-neb merged 3 commits intozed-industries:mainfrom
Conversation
When deleting a default keybinding, the keymap editor created a NoAction suppression entry that appeared as a phantom `<null>` row. When changing a default binding's keystroke, the old default continued showing under the original keystroke. Filter NoAction suppression bindings from displayed matches in the keymap editor, and append a null suppression for the old keystroke when replacing a non-user binding with a different keystroke. Closes zed-industries#48725 Liam
|
Hey, thanks for taking a crack at this. We really struggled with what to do here while building the keymap editor. For starters I'm a big fan of goal 2, I think that's a much needed fix to the existing behavior, and something we should've done from the start. I'm a bit worried about goal 1, however. Maybe a filter icon button would be OK. We can make the |
d26f6cf to
6c26b62
Compare
|
Hey @iam-liam -- congratulations on your first contribution to Zed! 💖 |
|
@probably-neb no problem, glad it's resolved. @yeskunall awesome, pleased to be involved. First of many, I hope! |
Summary
Fixes #48725
When using the keymap editor to delete or re-bind default keybindings, two issues caused stale entries to persist:
Deleting a default binding created a
NoActionsuppression entry inkeymap.json(correct for GPUI-level suppression), but the keymap editor displayed it as a phantom<null>row alongside the greyed-out default. Searching by keystroke showed both entries, and clash detection counted the phantom as a real binding.Changing a default binding's keystroke (e.g.
⌥⌘T→⇧⌃⌘T) added the new binding but never suppressed the original default under the old keystroke. The old default continued appearing in keystroke searches and clash warnings.Fix
Filter
NoActionsuppression bindings from the keymap editor display. These remain in the internal binding list for conflict detection (so overridden defaults are still correctly greyed out), but are excluded from the visible match results.Append a
nullsuppression for the old keystroke when replacing a non-user binding with a different keystroke. Whenupdate_keybindingconverts aReplacetoAddfor a non-user binding and the keystroke changes, it now also writes{"old-keystroke": null}(with the original context) to suppress the stale default.Reproduction steps (verified fixed)
⌥⌘T⇧⌃⌘T, click Save<null>phantom row appears, and no stale defaults remain under⌥⌘T⌥⌘T— no results⌥⌘T— no clash warningsTest plan
keymap_updateandtest_keymap_removetests passRelease Notes: