Skip to content

Terminal: new-tab, select-all and close-tab shortcuts - #22

Open
neo773 wants to merge 1 commit into
zeronsh:mainfrom
neo773:feat/terminal-shortcuts
Open

Terminal: new-tab, select-all and close-tab shortcuts#22
neo773 wants to merge 1 commit into
zeronsh:mainfrom
neo773:feat/terminal-shortcuts

Conversation

@neo773

@neo773 neo773 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Stacked on #21 (which is itself stacked on #20). The first three commits belong to those PRs and drop out of this diff as they merge. The commit to review here is 8e63b6f.

Chord Action Scope
Ctrl+Shift+` New terminal tab Global
Cmd/Ctrl+A Select all, scrollback included Terminal
Cmd/Ctrl+W Close active tab Terminal

New-tab opens the panel when it is closed and adds a tab when it is already open — set_open creates the first tab itself, so doing both would open two. Close-tab goes through the same path as middle-click and the tab's ×, so closing the last one still collapses the drawer and still tells the engine to reap the PTY.

Three things that fail silently if you get them wrong

Bindings must be registered from apply_keymap, not terminal::panel::init. That function clears the keymap, and Shell::new runs it during startup — so anything bound at app init is gone before the first frame. No error; the shortcut simply never fires. There is a test keeping these out of init.

Shifted punctuation is written as the character it produces. ctrl-shift-\`` parses, builds, and then never matches: the platform reports with no shift modifier, so a binding demanding both matches nothing. Letters are the opposite —ctrl-shift-a is right, because Shift stays a modifier. zed's keymaps show the same split (ctrl-, ctrl-?for punctuation;ctrl-shift-c, ctrl-shift-nfor letters). A test pins the chord to key~`, ctrl on, shift off.

macOS "Close Window" moves off ⌘W to ⌘⇧W. AppKit fires menu key equivalents in performKeyEquivalent:, before the key reaches any view, so a menu item holding ⌘W makes every view-level ⌘W binding unreachable. zed and Safari move it for the same reason.

Consequences worth flagging

  • ⌘W no longer closes the window. ⌘⇧W does, and that is now the action's only binding — gpui derives the menu equivalent from the keymap, and a second ⌘W binding risks handing the menu back the chord it just gave up. Outside the terminal, ⌘W does nothing.
  • On Windows and Linux, select-all and close-tab resolve to Ctrl+A and Ctrl+W, shadowing readline's beginning-of-line and delete-word inside the terminal. Same trade VS Code makes; shifted forms are the alternative if it grates.
  • New-tab keeps literal Ctrl on macOS rather than Cmd, matching VS Code and zed.

Scoping select-all and close-tab to the Terminal context is also what gives them precedence: gpui ranks matches by context depth before registration order.

Testing

comet-ui is 380/380 green, clippy and fmt clean. New coverage: select-all reaching into scrollback and not panicking on an empty grid; the binding table parsing, scoping, and resolving mod- per platform.

Verified in the running app that ⌘⇧W closes the window. The remaining chords are covered by unit tests but have not been through a manual pass end to end.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Ctrl+Shift+` opens a new tab (VS Code's chord), mod+A selects the buffer
including scrollback, and mod+W closes the active tab through the same path as
middle-click, so the last one still collapses the drawer and reaps the PTY.

Three things that each fail silently if you get them wrong:

Bindings have to be registered from `apply_keymap`, not `terminal::panel::init`.
That function clears the keymap and `Shell::new` runs it during startup, so
anything bound at app init is gone before the first frame.

Shifted punctuation is written as the character it produces. `ctrl-shift-\``
parses and builds, then never matches, because the platform reports `~` with no
shift modifier. Letters keep shift as a modifier; punctuation does not. zed's
keymaps show the same split.

macOS "Close Window" moves off ⌘W to ⌘⇧W. AppKit fires menu key equivalents in
`performKeyEquivalent:`, before the key reaches any view, so a menu item on ⌘W
makes every view-level ⌘W binding unreachable. zed and Safari move it for the
same reason. ⌘⇧W is now the action's only binding: gpui derives the menu
equivalent from the keymap, and a second ⌘W binding could hand the menu back
the chord it just gave up.

Select-all and close-tab are scoped to the Terminal context, which is also what
gives them precedence — gpui ranks matches by context depth before registration
order. On Windows and Linux they resolve to Ctrl+A and Ctrl+W and shadow
readline's beginning-of-line and delete-word inside the terminal, the same trade
VS Code makes.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@neo773
neo773 force-pushed the feat/terminal-shortcuts branch from 8e63b6f to 6d1ce75 Compare August 6, 2026 21:18
@neo773

neo773 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@wingleeio
Could I get a review on this when you get some time?

@wingleeio

Copy link
Copy Markdown
Contributor

@neo773 I'll review this and try to get it in tomorrow

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.

2 participants