fix(keybindings): preserve plugin bindings when switching keybinding maps (#2307)#2335
Merged
Conversation
…er (#2307) Plugin-contributed bindings (modes registered at runtime via defineMode) live only in the KeybindingResolver — its plugin_defaults, plugin_chord_defaults, and inheriting_modes fields — and are not represented in Config. Every config-triggered rebuild replaced the live resolver with a fresh `KeybindingResolver::new(&config)`, which starts those fields empty, so all plugin bindings silently vanished until the next restart. Switching the active keybinding map and back is the user-visible trigger (#2307): the editor reported 866 -> 547 bindings, dropping all 391 plugin-contributed entries. The same latent loss affected saving settings, editing keybindings, theme/toggle reloads, and external config reloads. Add `KeybindingResolver::reload_from_config`, which rebuilds the config-derived keymap and custom bindings exactly as `new` does but carries the runtime plugin state across untouched. Route all six rebuild sites through it instead of overwriting the resolver. Covered by an e2e test that drives the real SwitchKeybindingMap action round-trip and a resolver unit test for the reload contract (single keys, chords, and inheriting-modes membership all survive).
ac4459c to
16589f2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2307. Switching the active keybinding map and back dropped every plugin-contributed binding (the Keybinding Editor reported
866 → 547, losing all 391 plugin bindings) until the next restart.Root cause
Plugin bindings — modes registered at runtime via
defineMode— live only in theKeybindingResolver(plugin_defaults,plugin_chord_defaults,inheriting_modes). They are not part ofConfig. Every config-triggered rebuild replaced the live resolver with a freshKeybindingResolver::new(&config), which starts those fields empty, so the entire plugin layer silently vanished.Switching keybinding maps is the user-visible trigger, but the same latent loss affected saving settings, editing keybindings, theme/toggle reloads, and external config reloads — all six call sites did
*self.keybindings.write().unwrap() = KeybindingResolver::new(&self.config).Fix
Add
KeybindingResolver::reload_from_config, which rebuilds the config-derived keymap + custom bindings exactly asnewdoes, then carries the runtime plugin state across untouched. Route all six rebuild sites through it.Testing
test_switch_keybinding_map_preserves_plugin_bindings): registers a plugin mode binding, drives the realSwitchKeybindingMapaction toemacsand back todefault, and asserts the binding still resolves. Fails before the fix (left: None), passes after.test_reload_from_config_preserves_plugin_state): single keys, chords, and inheriting-modes membership all survivereload_from_config.keybinding_editore2e module: 53 passed.cargo fmt --check,cargo clippy -p fresh-editor --all-targetsclean for the touched code.https://claude.ai/code/session_01LnBJS6kx9TmSAnwsbz3Dnt
Generated by Claude Code