Skip to content

fix(keybindings): honor null overrides and terminal shortcut forwarding#2024

Merged
bajrangCoder merged 2 commits intoAcode-Foundation:mainfrom
bajrangCoder:fix/terminal-keybinding-overrides
Apr 10, 2026
Merged

fix(keybindings): honor null overrides and terminal shortcut forwarding#2024
bajrangCoder merged 2 commits intoAcode-Foundation:mainfrom
bajrangCoder:fix/terminal-keybinding-overrides

Conversation

@bajrangCoder
Copy link
Copy Markdown
Member

No description provided.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 10, 2026

Greptile Summary

This PR fixes two related keybinding bugs: (1) null key overrides in the user's keybinding file were silently ignored because the old bindingInfo?.key ?? command.key expression treated null as absent and fell back to the default key; and (2) the terminal's shortcut-forwarding logic used the static keyBindings import instead of the runtime-resolved bindings, so user customisations had no effect in the terminal. The fix introduces resolveBindingInfo/buildResolvedKeyBindingsSnapshot helpers, a version-counter cache, and a .toLowerCase() normalisation for case-insensitive key matching in the terminal handler.

Confidence Score: 5/5

Safe to merge — both bug fixes are correct and no regressions were found.

All findings are P2 style suggestions. The null-override fix correctly uses hasOwnProperty to distinguish an explicit null from an absent key. The terminal caching logic is sound: the -1 sentinel guarantees at least one recompute after the first rebuildKeymap call, and the version counter reliably invalidates the cache on every keybinding reload.

No files require special attention.

Important Files Changed

Filename Overview
src/cm/commandRegistry.js Adds resolveBindingInfo / buildResolvedKeyBindingsSnapshot to correctly propagate null-key overrides, a version counter for cache invalidation, and exports getResolvedKeyBindings / getResolvedKeyBindingsVersion for the terminal; logic is sound.
src/components/terminal/terminal.js Replaces the static keyBindings import with the new resolved-bindings API, adds version-based memoisation of parsed keybindings, and normalises key names to lowercase; the changes are correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[setKeyBindings called] --> B[loadCustomKeyBindings]
    B --> |file exists| C[resolvedKeyBindings = file contents]
    B --> |file missing| D[resolvedKeyBindings = keyBindings default]
    C --> E[rebuildKeymap]
    D --> E
    E --> F[buildResolvedKeyBindingsSnapshot\ncachedResolvedKeyBindings]
    F --> G[resolveBindingInfo per name]
    G --> |override === null| H[key: null — binding disabled]
    G --> |override is object| I[merged binding]
    G --> |no override| J[base keyBinding]
    E --> K[resolvedKeyBindingsVersion++]
    K --> L[commandMap updated\ncommand.key / description]
    L --> M[cachedKeymap rebuilt for CodeMirror]

    N[Terminal keypress] --> O[parseAppKeybindings]
    O --> P{version match?}
    P --> |yes| Q[return cached parsedAppKeybindings]
    P --> |no| R[getResolvedKeyBindings\ncachedResolvedKeyBindings]
    R --> S[parse + cache new bindings\nupdate version]
    S --> T{isAppKeybinding?}
    Q --> T
    T --> |yes| U[dispatch to document\nreturn false — terminal skips]
    T --> |no| V[return true — terminal handles normally]
Loading

Reviews (2): Last reviewed commit: "fix" | Re-trigger Greptile

@bajrangCoder

This comment was marked as outdated.

@bajrangCoder bajrangCoder merged commit 9adbf23 into Acode-Foundation:main Apr 10, 2026
7 checks passed
@bajrangCoder bajrangCoder deleted the fix/terminal-keybinding-overrides branch April 10, 2026 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant