Skip to content

refactor(ui): unify keybinding infrastructure and support string initialization#21776

Merged
scidomino merged 2 commits intomainfrom
tomm_combine
Mar 9, 2026
Merged

refactor(ui): unify keybinding infrastructure and support string initialization#21776
scidomino merged 2 commits intomainfrom
tomm_combine

Conversation

@scidomino
Copy link
Collaborator

@scidomino scidomino commented Mar 9, 2026

Summary

Refactor keybinding infrastructure into a unified ui/key package and enable string-based KeyBinding initialization.

Details

  • Consolidated keyBindings.ts, keyMatchers.ts, keybindingUtils.ts, and keyToAnsi.ts into packages/cli/src/ui/key/.
  • Updated KeyBinding class to support VS Code-style string patterns (e.g., new KeyBinding('ctrl+c')) with modifier validation.
  • Refactored defaultKeyBindings and tests to use the new string-based format.
  • Optimized keyToAnsi.ts with data-driven mapping and removed redundant logic.
  • Removed weird "documentation only" Command.REWIND. This is already documented elsewhere in keyboard-shortcuts.md.

Related Issues

Related to #21294

How to Validate

  • npm run typecheck -w packages/cli
  • npm test -w packages/cli
  • npm run docs:keybindings
  • Full npm run preflight

Pre-Merge Checklist

  • Updated relevant documentation and README (if needed)
  • Added/updated tests (if needed)
  • Noted breaking changes (if any)
  • Validated on required platforms/methods:
    • MacOS
      • npm run
      • npx
      • Docker
      • Podman
      • Seatbelt

@gemini-cli gemini-cli bot added priority/p1 Important and should be addressed in the near term. area/core Issues related to User Interface, OS Support, Core Functionality labels Mar 9, 2026
@github-actions
Copy link

github-actions bot commented Mar 9, 2026

Size Change: +1.93 kB (+0.01%)

Total Size: 26.1 MB

Filename Size Change
./bundle/gemini.js 25.7 MB +1.93 kB (+0.01%)
ℹ️ View Unchanged
Filename Size
./bundle/node_modules/@google/gemini-cli-devtools/dist/client/main.js 221 kB
./bundle/node_modules/@google/gemini-cli-devtools/dist/src/_client-assets.js 227 kB
./bundle/node_modules/@google/gemini-cli-devtools/dist/src/index.js 11.5 kB
./bundle/node_modules/@google/gemini-cli-devtools/dist/src/types.js 132 B
./bundle/sandbox-macos-permissive-open.sb 890 B
./bundle/sandbox-macos-permissive-proxied.sb 1.31 kB
./bundle/sandbox-macos-restrictive-open.sb 3.36 kB
./bundle/sandbox-macos-restrictive-proxied.sb 3.56 kB
./bundle/sandbox-macos-strict-open.sb 4.82 kB
./bundle/sandbox-macos-strict-proxied.sb 5.02 kB

compressed-size-action

@scidomino scidomino marked this pull request as ready for review March 9, 2026 22:16
@scidomino scidomino requested review from a team as code owners March 9, 2026 22:17
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors the keybinding system within the CLI, centralizing related logic into a new ui/key package. The core change introduces a KeyBinding class that enables string-based key definition, simplifying how keyboard shortcuts are configured and managed. This enhancement improves the clarity and maintainability of the keybinding system, making it easier to define, validate, and understand keyboard shortcuts across the application.

Highlights

  • Unified Keybinding Infrastructure: The keybinding-related files (keyBindings.ts, keyMatchers.ts, keybindingUtils.ts, keyToAnsi.ts) have been consolidated into a new, dedicated packages/cli/src/ui/key/ package for better organization and maintainability.
  • String-based KeyBinding Initialization: A new KeyBinding class has been introduced, allowing keybindings to be defined using VS Code-style string patterns (e.g., new KeyBinding('ctrl+c')), which simplifies configuration and includes modifier validation.
  • Refactored Keybinding Definitions: The defaultKeyBindings have been updated to leverage the new string-based KeyBinding class, making the definitions more concise and readable.
  • Optimized Key to ANSI Translation: The keyToAnsi.ts utility has been refactored to use a data-driven mapping for ANSI escape sequences, removing redundant logic and improving clarity.
  • Removed Redundant Command: The Command.REWIND entry, which was previously documentation-only, has been removed from the keybinding configuration and related documentation to streamline the command set.
Changelog
  • docs/reference/keyboard-shortcuts.md
    • Removed the 'Browse and rewind previous interactions' entry from the history & search section.
  • packages/cli/src/config/keyBindings.test.ts
    • Removed as its functionality was moved and integrated into the new keybinding infrastructure.
  • packages/cli/src/ui/AppContainer.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/auth/ApiAuthDialog.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/AdminSettingsChangedDialog.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/ApprovalModeIndicator.tsx
    • Updated import paths for formatCommand and Command to the new ../key/ directory.
  • packages/cli/src/ui/components/AskUserDialog.tsx
    • Updated import paths for Command and formatCommand to the new ../key/ directory.
  • packages/cli/src/ui/components/BackgroundShellDisplay.tsx
    • Updated import paths for Command and formatCommand to the new ../key/ directory.
  • packages/cli/src/ui/components/ExitPlanModeDialog.test.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/ExitPlanModeDialog.tsx
    • Updated import paths for Command and formatCommand to the new ../key/ directory.
  • packages/cli/src/ui/components/FooterConfigDialog.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/Help.tsx
    • Updated import paths for formatCommand and Command to the new ../key/ directory.
  • packages/cli/src/ui/components/HooksDialog.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/InputPrompt.test.tsx
    • Updated import path for defaultKeyMatchers and Command to the new ../key/ directory.
  • packages/cli/src/ui/components/InputPrompt.tsx
    • Updated import paths for Command and formatCommand to the new ../key/ directory.
  • packages/cli/src/ui/components/PolicyUpdateDialog.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/RawMarkdownIndicator.tsx
    • Updated import paths for formatCommand and Command to the new ../key/ directory.
  • packages/cli/src/ui/components/RewindConfirmation.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/RewindViewer.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/ShellInputPrompt.tsx
    • Updated import paths for keyToAnsi and Command to the new ../key/ directory.
  • packages/cli/src/ui/components/ShortcutsHelp.tsx
    • Updated import paths for Command and formatCommand to the new ../key/ directory.
    • Replaced formatCommand(Command.REWIND) with a literal 'Double Esc' for display.
  • packages/cli/src/ui/components/ValidationDialog.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/messages/Todo.tsx
    • Updated import paths for formatCommand and Command to the new ../key/ directory.
  • packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx
    • Updated import paths for Command and formatCommand to the new ../key/ directory.
  • packages/cli/src/ui/components/messages/ToolShared.tsx
    • Updated import paths for formatCommand and Command to the new ../key/ directory.
  • packages/cli/src/ui/components/shared/BaseSettingsDialog.tsx
    • Updated import paths for Command and formatCommand to the new ../key/ directory.
  • packages/cli/src/ui/components/shared/MaxSizedBox.tsx
    • Updated import paths for Command and formatCommand to the new ../key/ directory.
  • packages/cli/src/ui/components/shared/Scrollable.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/shared/ScrollableList.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/shared/SearchableList.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/shared/TextInput.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/shared/text-buffer.ts
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/triage/TriageDuplicates.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/components/triage/TriageIssues.tsx
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/hooks/keyToAnsi.ts
    • Removed as its functionality was moved and refactored into packages/cli/src/ui/key/keyToAnsi.ts.
  • packages/cli/src/ui/hooks/useApprovalModeIndicator.ts
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/hooks/useKeyMatchers.ts
    • Updated import paths for KeyMatchers and defaultKeyMatchers to the new ../key/ directory.
  • packages/cli/src/ui/hooks/useSelectionList.ts
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/hooks/useSuspend.test.ts
    • Updated import paths for formatCommand and Command to the new ../key/ directory.
  • packages/cli/src/ui/hooks/useSuspend.ts
    • Updated import paths for formatCommand and Command to the new ../key/ directory.
  • packages/cli/src/ui/hooks/useTabbedNavigation.test.ts
    • Removed mocks for useKeyMatchers and Command as the new KeyBinding class handles matching directly.
  • packages/cli/src/ui/hooks/useTabbedNavigation.ts
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/hooks/vim.ts
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • packages/cli/src/ui/key/keyBindings.test.ts
    • Added new test file for the KeyBinding class, including tests for its constructor and modifier parsing.
    • Included existing tests for defaultKeyBindings and command metadata.
  • packages/cli/src/ui/key/keyBindings.ts
    • Renamed from packages/cli/src/config/keyBindings.ts.
    • Refactored KeyBinding from an interface to a class, enabling string-based initialization and matches method.
    • Removed Command.REWIND from the Command enum.
    • Updated defaultKeyBindings to use the new KeyBinding class for all key definitions.
  • packages/cli/src/ui/key/keyMatchers.test.ts
    • Renamed from packages/cli/src/ui/keyMatchers.test.ts.
    • Updated imports to reflect the new ui/key package structure.
    • Modified test cases to use the KeyBinding class for custom configurations.
  • packages/cli/src/ui/key/keyMatchers.ts
    • Renamed from packages/cli/src/ui/keyMatchers.ts.
    • Removed the matchKeyBinding function, delegating matching logic to the KeyBinding class's matches method.
    • Updated imports to reflect the new ui/key package structure.
  • packages/cli/src/ui/key/keyToAnsi.ts
    • Added new file, refactored from packages/cli/src/ui/hooks/keyToAnsi.ts.
    • Implemented a data-driven SPECIAL_KEYS map for ANSI escape sequence translation.
  • packages/cli/src/ui/key/keybindingUtils.test.ts
    • Renamed from packages/cli/src/ui/utils/keybindingUtils.test.ts.
    • Updated imports to reflect the new ui/key package structure.
    • Modified test cases to use the KeyBinding class for formatting tests.
  • packages/cli/src/ui/key/keybindingUtils.ts
    • Renamed from packages/cli/src/ui/utils/keybindingUtils.ts.
    • Updated imports to reflect the new ui/key package structure.
    • Removed 'double escape' from the KEY_NAME_MAP.
  • packages/cli/src/ui/utils/shortcutsHelp.ts
    • Updated import path for Command from ../keyMatchers.js to ../key/keyMatchers.js.
  • scripts/generate-keybindings-doc.ts
    • Updated import paths for KeyBinding, commandCategories, commandDescriptions, defaultKeyBindings, and formatKeyBinding to the new ui/key directory.
Activity
  • The author, scidomino, initiated this pull request to refactor the keybinding infrastructure.
  • The changes involve significant file reorganizations and code refactoring, as detailed in the highlights and changelog.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request is a solid refactoring that unifies the keybinding infrastructure and introduces a more intuitive string-based initialization for KeyBinding. The code is much cleaner and more maintainable. I've found one critical issue with the new KeyBinding class constructor that could lead to incorrect parsing for certain key patterns.

Copy link
Contributor

@jacob314 jacob314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be worth it to bring back/ make first class double keypress bindings for real at some point. think they are generally important for cases like ctrl-C twice, etc.

@scidomino scidomino enabled auto-merge March 9, 2026 23:14
@scidomino scidomino added this pull request to the merge queue Mar 9, 2026
Merged via the queue into main with commit 215f8f3 Mar 9, 2026
26 of 27 checks passed
@scidomino scidomino deleted the tomm_combine branch March 9, 2026 23:42
@jacob314
Copy link
Contributor

This review is provided by /review-frontend.

1. Architectural Rule Violation (Crucial)

The PR moves keyBindings.ts from packages/cli/src/config/keyBindings.ts to packages/cli/src/ui/key/keyBindings.ts. This currently violates an explicit mandate in both packages/cli/GEMINI.md and .gemini/commands/strict-development-rules.md, which state:

"Shortcuts: only define keyboard shortcuts in packages/cli/src/config/keyBindings.ts"

Note from Jacob: This indicates that the /review-frontend rules are out of date, and we need to update the .gemini/GEMINI.md and strict-development-rules.md files to cover the new locations.

2. Constructor Parsing Bug (The + key edge case)

The new KeyBinding class constructor introduces string parsing using .split('+'):

const parts = pattern.toLowerCase().split('+');
const key = parts.pop()!;

If a user configures a keybinding that includes the actual + character (e.g., ctrl++), the split yields ['ctrl', '', '']. This sets the key to '' and evaluates '' as an invalid modifier, throwing an error. This will limit the extensibility of user-defined shortcuts.

3. Data-driven Configuration vs. Classes

The KeyBindingConfig has JSDoc stating it is a "Data-driven key binding structure for user configuration". The PR changes KeyBinding from a pure interface to a class. If these configurations are meant to be loaded from user settings JSON files down the road, relying on instances of a class will require explicit instantiation steps rather than purely parsing JSON objects. It might be better to keep the interface and use a factory/parser function instead.

4. Flawed String Comparison in keyToAnsi.ts (Existing bug, but exposed)

In the newly consolidated keyToAnsi.ts (which previously lived in hooks/), there is a dangerous string comparison bug:

if (key.ctrl) {
  if (key.name >= 'a' && key.name <= 'z') { ... }
}

In JavaScript, multi-character strings like 'escape' or 'right' satisfy this condition ('escape' >= 'a' is true, and 'escape' <= 'z' is true). As a result, pressing Ctrl+Right incorrectly maps to Ctrl+R (\x12), and Ctrl+Escape maps to Ctrl+E (\x05). The check needs to be tightened to key.name.length === 1 && key.name >= 'a' && key.name <= 'z'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/core Issues related to User Interface, OS Support, Core Functionality priority/p1 Important and should be addressed in the near term.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants