feat: add quick terminal float window with double-ESC trigger#3205
Open
PureLo wants to merge 2 commits intowavetermdev:mainfrom
Open
feat: add quick terminal float window with double-ESC trigger#3205PureLo wants to merge 2 commits intowavetermdev:mainfrom
PureLo wants to merge 2 commits intowavetermdev:mainfrom
Conversation
- Implement quick terminal float window triggered by double-ESC (300ms threshold) - Auto-inherit cwd and connection context from focused block - Adaptive height: starts at 10% window, grows with content up to 50% max - Position: uses source block width with horizontal inset, falls back to layout width - Add quickTerminalAtom for state management (visible/blockId/opening/closing) - ESC key: single ESC dismisses quick terminal, passthrough vdom mode escape to global - OSC 7: track current working directory to currentCwdAtom for context inheritance - Add ephemeral 'quick-terminal' node type with dynamic height calculation
Contributor
WalkthroughAdds a quick-terminal feature across state, input, view, and layout layers. Introduces a new global Jotai atom Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
Author
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/app/store/keymodel.ts (1)
731-769:⚠️ Potential issue | 🟠 MajorDon't arm double-ESC on UI-consumed
Escape.
lastEscapeTimeis set before the modal/search branches run, so anEscapethat only closes a modal or dismisses search becomes the first half of a “double-ESC”. The next normalEscapewithin 300ms will summon the quick terminal unexpectedly. Only update the timer when the key is otherwise unhandled, and clear it on the modal/search paths.Suggested fix
globalKeyMap.set("Escape", () => { const now = Date.now(); const quickTermState = globalStore.get(atoms.quickTerminalAtom); // Handle quick terminal toggle on double-ESC if (quickTermState.visible) { // If quick terminal is open, single ESC dismisses it // Skip if already closing to prevent double-close if (!quickTermState.closing) { fireAndForget(() => toggleQuickTerminal()); } lastEscapeTime = 0; // Reset to prevent stale double-ESC detection return true; } if (quickTermState.opening || quickTermState.closing) { lastEscapeTime = 0; return true; } + if (modalsModel.hasOpenModals()) { + lastEscapeTime = 0; + modalsModel.popModal(); + return true; + } + if (deactivateSearch()) { + lastEscapeTime = 0; + return true; + } + // Check for double-ESC to summon quick terminal if (now - lastEscapeTime < QUICK_TERM_DOUBLE_ESC_TIMEOUT) { // Double ESC detected - summon quick terminal fireAndForget(() => toggleQuickTerminal()); lastEscapeTime = 0; // Reset after handling return true; } lastEscapeTime = now; - - // Existing ESC behavior (modals, search) - if (modalsModel.hasOpenModals()) { - modalsModel.popModal(); - return true; - } - if (deactivateSearch()) { - return true; - } return false; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/store/keymodel.ts` around lines 731 - 769, The Escape handler currently sets lastEscapeTime too early, so UI-consumed Escapes (modal close via modalsModel.popModal or search dismiss via deactivateSearch) arm the double-ESC timer; change the flow in the globalKeyMap.set("Escape", ...) handler so lastEscapeTime is only set when the key was not otherwise handled: 1) remove the early lastEscapeTime = now assignment; 2) when a quick terminal is visible/closing/opening keep/reset lastEscapeTime as already done; 3) when you handle UI actions (modalsModel.popModal() or deactivateSearch()) explicitly clear lastEscapeTime (set to 0) and return true; and 4) only after those checks, if no UI handling occurred, set lastEscapeTime = now and then run the double-ESC detection against QUICK_TERM_DOUBLE_ESC_TIMEOUT to summon toggleQuickTerminal().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/store/global.ts`:
- Around line 711-721: The quick-terminal state may remain stuck with
closing=true if layoutModel.closeNode or ObjectService.DeleteBlock throws, so
wrap the teardown in a try/finally: after reading quickTermState and setting
globalStore.set(atoms.quickTerminalAtom, { ...quickTermState, closing: true }),
perform the async closeNode/deleteBlock inside try and in the finally always
reset the atom to QuickTerminalInitialState via
globalStore.set(atoms.quickTerminalAtom, QuickTerminalInitialState); ensure you
reference quickTermState.blockId, layoutModel.getNodeByBlockId,
layoutModel.closeNode, ObjectService.DeleteBlock, and QuickTerminalInitialState
when making the change.
In `@frontend/layout/lib/layoutModel.ts`:
- Around line 76-77: Remove the two unused constants QuickTerminalFallbackCols
and QuickTerminalFallbackCharWidthPx from layoutModel.ts to eliminate dead code;
locate the const declarations for QuickTerminalFallbackCols and
QuickTerminalFallbackCharWidthPx in layoutModel.ts (or wherever they appear) and
delete those lines, then run the project type-check/build and tests to confirm
nothing else references them and commit the change.
---
Outside diff comments:
In `@frontend/app/store/keymodel.ts`:
- Around line 731-769: The Escape handler currently sets lastEscapeTime too
early, so UI-consumed Escapes (modal close via modalsModel.popModal or search
dismiss via deactivateSearch) arm the double-ESC timer; change the flow in the
globalKeyMap.set("Escape", ...) handler so lastEscapeTime is only set when the
key was not otherwise handled: 1) remove the early lastEscapeTime = now
assignment; 2) when a quick terminal is visible/closing/opening keep/reset
lastEscapeTime as already done; 3) when you handle UI actions
(modalsModel.popModal() or deactivateSearch()) explicitly clear lastEscapeTime
(set to 0) and return true; and 4) only after those checks, if no UI handling
occurred, set lastEscapeTime = now and then run the double-ESC detection against
QUICK_TERM_DOUBLE_ESC_TIMEOUT to summon toggleQuickTerminal().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bf5e1fbe-9e98-40da-b13b-00ebcdd469f3
📒 Files selected for processing (9)
frontend/app/store/global-atoms.tsfrontend/app/store/global.tsfrontend/app/store/keymodel.tsfrontend/app/view/term/osc-handlers.tsfrontend/app/view/term/term-model.tsfrontend/app/view/term/termwrap.tsfrontend/layout/lib/layoutModel.tsfrontend/preview/mock/mockwaveenv.tsfrontend/types/custom.d.ts
Comment on lines
+711
to
+721
| if (quickTermState.visible && quickTermState.blockId) { | ||
| // Dismiss: close the ephemeral node | ||
| // Set closing flag to prevent race condition with double-ESC | ||
| globalStore.set(atoms.quickTerminalAtom, { ...quickTermState, closing: true }); | ||
| const quickTerminalNode = layoutModel.getNodeByBlockId(quickTermState.blockId); | ||
| if (quickTerminalNode != null) { | ||
| await layoutModel.closeNode(quickTerminalNode.id); | ||
| } else { | ||
| await ObjectService.DeleteBlock(quickTermState.blockId); | ||
| } | ||
| globalStore.set(atoms.quickTerminalAtom, QuickTerminalInitialState); |
Contributor
There was a problem hiding this comment.
Always clear quick-terminal state when dismiss fails.
If layoutModel.closeNode() or ObjectService.DeleteBlock() throws after Line 714, quickTerminalAtom.closing never gets reset. frontend/app/store/keymodel.ts Line 746 then swallows subsequent Escape presses, effectively bricking the quick terminal until reload. Move the reset into a finally block so teardown errors cannot leave the state stuck.
Suggested fix
if (quickTermState.visible && quickTermState.blockId) {
// Dismiss: close the ephemeral node
// Set closing flag to prevent race condition with double-ESC
globalStore.set(atoms.quickTerminalAtom, { ...quickTermState, closing: true });
- const quickTerminalNode = layoutModel.getNodeByBlockId(quickTermState.blockId);
- if (quickTerminalNode != null) {
- await layoutModel.closeNode(quickTerminalNode.id);
- } else {
- await ObjectService.DeleteBlock(quickTermState.blockId);
- }
- globalStore.set(atoms.quickTerminalAtom, QuickTerminalInitialState);
+ try {
+ const quickTerminalNode = layoutModel.getNodeByBlockId(quickTermState.blockId);
+ if (quickTerminalNode != null) {
+ await layoutModel.closeNode(quickTerminalNode.id);
+ } else {
+ await ObjectService.DeleteBlock(quickTermState.blockId);
+ }
+ } finally {
+ globalStore.set(atoms.quickTerminalAtom, QuickTerminalInitialState);
+ }
return true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (quickTermState.visible && quickTermState.blockId) { | |
| // Dismiss: close the ephemeral node | |
| // Set closing flag to prevent race condition with double-ESC | |
| globalStore.set(atoms.quickTerminalAtom, { ...quickTermState, closing: true }); | |
| const quickTerminalNode = layoutModel.getNodeByBlockId(quickTermState.blockId); | |
| if (quickTerminalNode != null) { | |
| await layoutModel.closeNode(quickTerminalNode.id); | |
| } else { | |
| await ObjectService.DeleteBlock(quickTermState.blockId); | |
| } | |
| globalStore.set(atoms.quickTerminalAtom, QuickTerminalInitialState); | |
| if (quickTermState.visible && quickTermState.blockId) { | |
| // Dismiss: close the ephemeral node | |
| // Set closing flag to prevent race condition with double-ESC | |
| globalStore.set(atoms.quickTerminalAtom, { ...quickTermState, closing: true }); | |
| try { | |
| const quickTerminalNode = layoutModel.getNodeByBlockId(quickTermState.blockId); | |
| if (quickTerminalNode != null) { | |
| await layoutModel.closeNode(quickTerminalNode.id); | |
| } else { | |
| await ObjectService.DeleteBlock(quickTermState.blockId); | |
| } | |
| } finally { | |
| globalStore.set(atoms.quickTerminalAtom, QuickTerminalInitialState); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/store/global.ts` around lines 711 - 721, The quick-terminal
state may remain stuck with closing=true if layoutModel.closeNode or
ObjectService.DeleteBlock throws, so wrap the teardown in a try/finally: after
reading quickTermState and setting globalStore.set(atoms.quickTerminalAtom, {
...quickTermState, closing: true }), perform the async closeNode/deleteBlock
inside try and in the finally always reset the atom to QuickTerminalInitialState
via globalStore.set(atoms.quickTerminalAtom, QuickTerminalInitialState); ensure
you reference quickTermState.blockId, layoutModel.getNodeByBlockId,
layoutModel.closeNode, ObjectService.DeleteBlock, and QuickTerminalInitialState
when making the change.
Comment on lines
+76
to
+77
| const QuickTerminalFallbackCols = 80; | ||
| const QuickTerminalFallbackCharWidthPx = 8; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for usage of these constants across the codebase
rg -n "QuickTerminalFallbackCols|QuickTerminalFallbackCharWidthPx" --type=tsRepository: wavetermdev/waveterm
Length of output: 221
Remove unused constants to avoid dead code.
QuickTerminalFallbackCols and QuickTerminalFallbackCharWidthPx are defined but never used anywhere in the codebase. Remove them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/layout/lib/layoutModel.ts` around lines 76 - 77, Remove the two
unused constants QuickTerminalFallbackCols and QuickTerminalFallbackCharWidthPx
from layoutModel.ts to eliminate dead code; locate the const declarations for
QuickTerminalFallbackCols and QuickTerminalFallbackCharWidthPx in layoutModel.ts
(or wherever they appear) and delete those lines, then run the project
type-check/build and tests to confirm nothing else references them and commit
the change.
- Replace `as any` casts with proper `TermViewModel` type casting - Extract "quick-terminal" magic string to QuickTerminalEphemeralType constant
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/app/store/global.ts (1)
709-719:⚠️ Potential issue | 🟠 MajorAlways reset quick-terminal state in a
finally.If
closeNode()orObjectService.DeleteBlock()throws after Line 712, the atom stays stuck withclosing: true, and subsequentEscapepresses keep getting swallowed.Suggested fix
globalStore.set(atoms.quickTerminalAtom, { ...quickTermState, closing: true }); - const quickTerminalNode = layoutModel.getNodeByBlockId(quickTermState.blockId); - if (quickTerminalNode != null) { - await layoutModel.closeNode(quickTerminalNode.id); - } else { - await ObjectService.DeleteBlock(quickTermState.blockId); + try { + const quickTerminalNode = layoutModel.getNodeByBlockId(quickTermState.blockId); + if (quickTerminalNode != null) { + await layoutModel.closeNode(quickTerminalNode.id); + } else { + await ObjectService.DeleteBlock(quickTermState.blockId); + } + } finally { + globalStore.set(atoms.quickTerminalAtom, QuickTerminalInitialState); } - globalStore.set(atoms.quickTerminalAtom, QuickTerminalInitialState); return true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/store/global.ts` around lines 709 - 719, The quick-terminal atom can remain stuck with closing: true if layoutModel.closeNode or ObjectService.DeleteBlock throws; wrap the async close/delete logic starting from the check of quickTermState.blockId (the calls to layoutModel.getNodeByBlockId, layoutModel.closeNode, and ObjectService.DeleteBlock) in a try/finally and move the final globalStore.set(atoms.quickTerminalAtom, QuickTerminalInitialState) into the finally block so the QuickTerminalInitialState is always restored even on exceptions; ensure you still set the intermediate closing: true state on quickTerminalAtom before the try block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/layout/lib/layoutModel.ts`:
- Around line 1352-1355: The code calls closeNode(...) inside
newQuickTerminalNode without awaiting it, which leaves rejections from async
closeNode detached and allows the previous ephemeral cleanup to race the new
quick-terminal creation; update newQuickTerminalNode to await the promise
returned by closeNode(this.getter(this.ephemeralNode).id) (and/or explicitly
catch errors) so the previous ephemeral node is fully closed before continuing,
and propagate this change to callers (notably the invocation in
frontend/app/store/global.ts) so they handle the revised async behavior.
---
Duplicate comments:
In `@frontend/app/store/global.ts`:
- Around line 709-719: The quick-terminal atom can remain stuck with closing:
true if layoutModel.closeNode or ObjectService.DeleteBlock throws; wrap the
async close/delete logic starting from the check of quickTermState.blockId (the
calls to layoutModel.getNodeByBlockId, layoutModel.closeNode, and
ObjectService.DeleteBlock) in a try/finally and move the final
globalStore.set(atoms.quickTerminalAtom, QuickTerminalInitialState) into the
finally block so the QuickTerminalInitialState is always restored even on
exceptions; ensure you still set the intermediate closing: true state on
quickTerminalAtom before the try block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 40551ae6-d1c2-4f8f-9875-1fa48d53ca0f
📒 Files selected for processing (2)
frontend/app/store/global.tsfrontend/layout/lib/layoutModel.ts
Comment on lines
+1352
to
+1355
| newQuickTerminalNode(blockId: string, sourceBlockId?: string | null) { | ||
| if (this.getter(this.ephemeralNode)) { | ||
| this.closeNode(this.getter(this.ephemeralNode).id); | ||
| } |
Contributor
There was a problem hiding this comment.
Await the previous ephemeral close before installing the quick terminal.
closeNode() is async. Calling it bare here detaches any rejection from onNodeDelete() and lets the previous ephemeral cleanup race the new quick-terminal creation.
Suggested fix
- newQuickTerminalNode(blockId: string, sourceBlockId?: string | null) {
- if (this.getter(this.ephemeralNode)) {
- this.closeNode(this.getter(this.ephemeralNode).id);
+ async newQuickTerminalNode(blockId: string, sourceBlockId?: string | null) {
+ const existingEphemeralNode = this.getter(this.ephemeralNode);
+ if (existingEphemeralNode) {
+ await this.closeNode(existingEphemeralNode.id);
}Caller update needed in frontend/app/store/global.ts:
- layoutModel.newQuickTerminalNode(blockId, focusedBlockId);
+ await layoutModel.newQuickTerminalNode(blockId, focusedBlockId);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| newQuickTerminalNode(blockId: string, sourceBlockId?: string | null) { | |
| if (this.getter(this.ephemeralNode)) { | |
| this.closeNode(this.getter(this.ephemeralNode).id); | |
| } | |
| async newQuickTerminalNode(blockId: string, sourceBlockId?: string | null) { | |
| const existingEphemeralNode = this.getter(this.ephemeralNode); | |
| if (existingEphemeralNode) { | |
| await this.closeNode(existingEphemeralNode.id); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/layout/lib/layoutModel.ts` around lines 1352 - 1355, The code calls
closeNode(...) inside newQuickTerminalNode without awaiting it, which leaves
rejections from async closeNode detached and allows the previous ephemeral
cleanup to race the new quick-terminal creation; update newQuickTerminalNode to
await the promise returned by closeNode(this.getter(this.ephemeralNode).id)
(and/or explicitly catch errors) so the previous ephemeral node is fully closed
before continuing, and propagate this change to callers (notably the invocation
in frontend/app/store/global.ts) so they handle the revised async behavior.
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
2 participants
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
Add Quick Terminal float window feature — a lightweight ephemeral terminal summoned via
double-ESC, designed for quick commands without disrupting your workflow.
Motivation
Users frequently need a temporary terminal to run a quick command while working in the main
terminal. Creating a new block or tab is disruptive. Quick Terminal provides instant access via a
familiar hotkey, with automatic context inheritance.
Key Use Case: Non-Disruptive Commands During Claude Code Sessions
When using Claude Code in the main terminal, you often need to run auxiliary commands (e.g.,
git status,ls, checking env vars) without:Quick Terminal lets you press double-ESC, run your command, then ESC to return — all without
interrupting Claude Code's state.
Changes
Core Feature: Quick Terminal Float Window
frontend/app/store/global-atoms.tsquickTerminalAtomfor managing float windowfrontend/app/store/global.tstoggleQuickTerminal(),getInheritedContextFromBlock()frontend/app/store/keymodel.tsfrontend/app/view/term/osc-handlers.tscurrentCwdAtomfrontend/app/view/term/term-model.tsfrontend/app/view/term/termwrap.tscurrentCwdAtom,contentHeightRows,syncQuickTerminalHeight()frontend/layout/lib/layoutModel.tsnewQuickTerminalNode(),updateQuickTerminalNodeProps()frontend/preview/mock/mockwaveenv.tsquickTerminalAtomfrontend/types/custom.d.tsephemeralType,quickTerminalSourceBlockIdBehavior
ESCtwice within 300ms to open quick terminalESCwhile quick terminal is visible closes itcwdandconnectionfrom focused blockTechnical Details
quickTerminalAtommanages 4 states:visible,blockId,opening,closingsyncQuickTerminalHeight_debounced()Test Plan
Files Changed
frontend/app/store/global-atoms.ts | 7 ++
frontend/app/store/global.ts | 83 ++++++++++++++
frontend/app/store/keymodel.ts | 49 +++++++++---
frontend/app/view/term/osc-handlers.ts | 4 +-
frontend/app/view/term/term-model.ts | 4 ++
frontend/app/view/term/termwrap.ts | 30 ++++++-
frontend/layout/lib/layoutModel.ts |125 ++++++++++++++++++++++++++---
frontend/preview/mock/mockwaveenv.ts | 1 +
frontend/types/custom.d.ts | 8 +++
9 files changed, 293 insertions(+), 18 deletions(-)