Skip to content

keymap_editor: Fix default binding deletion and override#50699

Merged
probably-neb merged 3 commits intozed-industries:mainfrom
iam-liam:Keymap-editor-defaults
Mar 17, 2026
Merged

keymap_editor: Fix default binding deletion and override#50699
probably-neb merged 3 commits intozed-industries:mainfrom
iam-liam:Keymap-editor-defaults

Conversation

@iam-liam
Copy link
Copy Markdown
Contributor

@iam-liam iam-liam commented Mar 4, 2026

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 NoAction suppression entry in keymap.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

  1. Filter NoAction suppression 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.

  2. Append a null suppression for the old keystroke when replacing a non-user binding with a different keystroke. When update_keybinding converts a Replace to Add for 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)

  1. Open the keymap editor
  2. Click Search by Keystroke and press ⌥⌘T
  3. You should see "agent: new thread" and "pane: close other items" (both Default)
  4. Right-click "agent: new thread" and choose Delete
  5. Double-click "pane: close other items", change keystroke to ⇧⌃⌘T, click Save
  6. Verify no <null> phantom row appears, and no stale defaults remain under ⌥⌘T
  7. Close and reopen the keymap editor, search ⌥⌘T — no results
  8. Search for "editor: wrap selections in tag" and assign ⌥⌘T — no clash warnings

Test plan

  • Existing keymap_update and test_keymap_remove tests pass
  • Added test: replacing a non-user binding without changing the keystroke produces no suppression
  • Added test: replacing a non-user binding with context and keystroke change produces a suppression that preserves the context
  • Manual verification of all reproduction steps above

Release Notes:

  • Fixed keymap editor showing phantom entries and stale defaults when deleting or overriding default keybindings (#48725)

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
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Mar 4, 2026
@zed-community-bot zed-community-bot bot added the first contribution the author's first pull request to Zed. NOTE: the label application is automated via github actions label Mar 4, 2026
@iam-liam iam-liam marked this pull request as draft March 4, 2026 13:16
@iam-liam iam-liam marked this pull request as ready for review March 4, 2026 13:57
@zelenenka zelenenka added the guild Pull requests by someone in Zed Guild. NOTE: the label application is automated via github actions label Mar 16, 2026
@probably-neb
Copy link
Copy Markdown
Collaborator

probably-neb commented Mar 17, 2026

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 Edit in JSON and Create binding buttons into Icon buttons like the other buttons, I really just wanted it to be a "filter" button, and with that realization I realized we really want to organize the disparate filters we have into a dedicated submenu for them. That's what I've now pushed to your branch, sorry for co-opting your PR, I hope you're happy with the design I settled on. It should allow for hiding the NoAction entries pretty easily.

@probably-neb probably-neb force-pushed the Keymap-editor-defaults branch from d26f6cf to 6c26b62 Compare March 17, 2026 04:46
@probably-neb probably-neb enabled auto-merge (squash) March 17, 2026 04:50
@probably-neb probably-neb merged commit 94400d8 into zed-industries:main Mar 17, 2026
29 checks passed
@yeskunall
Copy link
Copy Markdown
Member

Hey @iam-liam -- congratulations on your first contribution to Zed! 💖

@iam-liam
Copy link
Copy Markdown
Contributor Author

@probably-neb no problem, glad it's resolved.

@yeskunall awesome, pleased to be involved. First of many, I hope!

@iam-liam iam-liam deleted the Keymap-editor-defaults branch March 19, 2026 15:28
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 first contribution the author's first pull request to Zed. NOTE: the label application is automated via github actions guild Pull requests by someone in Zed Guild. NOTE: the label application is automated via github actions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keymap editor does not delete or override defaults correctly

5 participants