feat(desktop): add click-to-move cursor and selection delete for terminal#357
feat(desktop): add click-to-move cursor and selection delete for terminal#357
Conversation
…inal Add two textarea-like features to the terminal: 1. Click-to-move cursor: Click anywhere on the current prompt line to move the cursor to that position. Works by sending arrow key sequences. 2. Selection delete on type: When text is selected on the prompt line and you type a character, the selection is deleted first and replaced with the typed character (like a textarea). Both features only work on the current cursor line (the editable prompt) and won't interfere with full-screen apps like vim. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded@Kitenite has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 18 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe pull request enhances the terminal component with a controlled write pathway via Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 Preview resources have been processed for cleanup |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx(3 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting, running at root level for speed
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid
anytype and prioritize type safety in TypeScript code
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.{ts,tsx}: Structure project folders as one folder per component with PascalCase naming (ComponentName/ComponentName.tsx + index.ts barrel export)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Use one component per file (no multi-component files)
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods from renderer process using window.ipcRenderer.invoke with type-safe object parameters
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-12T05:45:09.673Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.673Z
Learning: Applies to apps/desktop/src/main/lib/*.ts : Implement IPC handlers accepting object parameters (not positional parameters) in apps/desktop/src/main/lib/*.ts files
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (2)
setupKeyboardHandler(317-385)setupClickToMoveCursor(480-526)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Deploy Web
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Docs
- GitHub Check: Deploy API
- GitHub Check: Build
🔇 Additional comments (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (2)
14-22: Import/usage wiring for click-to-move looks consistent.
383-401: Cleanup ordering/includes look good (click handler removed).apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (2)
177-184:KeyboardHandlerOptions.onWriteaddition is a clean extension point.
317-326: Ordering (selection-delete before shortcuts) makes sense.
Just ensure the restart semantics are handled when exited (Terminal.tsx comment).
| /** | ||
| * Handle selection delete: when text is selected and user types a printable character, | ||
| * delete the selection first (like a text area), then insert the character. | ||
| * | ||
| * Returns true if the event was handled, false otherwise. | ||
| */ | ||
| function handleSelectionDelete( | ||
| xterm: XTerm, | ||
| event: KeyboardEvent, | ||
| onWrite?: (data: string) => void, | ||
| ): boolean { | ||
| if (!onWrite) return false; | ||
|
|
||
| // Only handle keydown events | ||
| if (event.type !== "keydown") return false; | ||
|
|
||
| // Only handle single printable characters (no modifiers except shift for capitals) | ||
| if (event.key.length !== 1) return false; | ||
| if (event.ctrlKey || event.metaKey || event.altKey) return false; | ||
|
|
||
| // Check if there's a selection | ||
| const selection = xterm.getSelection(); | ||
| if (!selection || selection.length === 0) return false; | ||
|
|
||
| const selectionPosition = xterm.getSelectionPosition(); | ||
| if (!selectionPosition) return false; | ||
|
|
||
| const buffer = xterm.buffer.active; | ||
| const cursorY = buffer.cursorY; | ||
| const cursorX = buffer.cursorX; | ||
|
|
||
| // Only handle selections on the current cursor line (prompt line) | ||
| // Both start and end of selection must be on the same line as cursor | ||
| const viewportCursorY = cursorY; | ||
| if ( | ||
| selectionPosition.start.y !== viewportCursorY || | ||
| selectionPosition.end.y !== viewportCursorY | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| const selStartX = selectionPosition.start.x; | ||
| const selEndX = selectionPosition.end.x; | ||
| const selectionLength = selEndX - selStartX; | ||
|
|
||
| if (selectionLength <= 0) return false; | ||
|
|
||
| // Prevent default handling - we'll handle this ourselves | ||
| event.preventDefault(); | ||
|
|
||
| // Build the sequence of operations: | ||
| // 1. Move cursor to selection start | ||
| // 2. Delete the selection (using delete key) | ||
| // 3. Type the new character | ||
|
|
||
| let sequence = ""; | ||
|
|
||
| // Move cursor to selection start | ||
| const moveToStart = selStartX - cursorX; | ||
| if (moveToStart !== 0) { | ||
| const arrowKey = moveToStart > 0 ? "\x1b[C" : "\x1b[D"; | ||
| sequence += arrowKey.repeat(Math.abs(moveToStart)); | ||
| } | ||
|
|
||
| // Delete the selection using Delete key escape sequence (\x1b[3~) | ||
| // This deletes characters forward from cursor position | ||
| sequence += "\x1b[3~".repeat(selectionLength); | ||
|
|
||
| // Type the new character | ||
| sequence += event.key; | ||
|
|
||
| // Clear the selection and send the sequence | ||
| xterm.clearSelection(); | ||
| onWrite(sequence); | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the full helpers.ts file to understand the context
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts | head -100Repository: superset-sh/superset
Length of output: 3999
🏁 Script executed:
# Check the file size and read the function in question (lines 230-306)
wc -l apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsRepository: superset-sh/superset
Length of output: 175
🏁 Script executed:
# Search for XTerm type definitions or imports in the codebase
rg "XTerm" apps/desktop/src/renderer --type ts --type tsx -A 2 -B 2 | head -80Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Look for xterm.js package and its version to check documentation
rg "xterm" package.json apps/desktop -A 5 -B 5Repository: superset-sh/superset
Length of output: 50376
🏁 Script executed:
# Read lines 230-306 of the helpers.ts file to see the function
sed -n '230,306p' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsRepository: superset-sh/superset
Length of output: 2343
🏁 Script executed:
# Search for other uses of getSelectionPosition in the codebase
rg "getSelectionPosition" apps/desktop/src --type ts -B 3 -A 3Repository: superset-sh/superset
Length of output: 1052
🏁 Script executed:
# Search for buffer property usage with xterm
rg "\.buffer\." apps/desktop/src --type ts -B 2 -A 2 | head -50Repository: superset-sh/superset
Length of output: 4505
🌐 Web query:
xterm.js buffer.normal buffer.active difference alternate buffer
💡 Result:
Short answer
- buffer.normal — the primary (normal) screen buffer (with scrollback). [1][2]
- buffer.alternate — the alternate (ALT) screen used by full‑screen apps (vim/less); it's separate, usually has no scrollback and is cleared/restored on exit. [3][4]
- buffer.active — a pointer to whichever buffer is currently in use (either normal or alternate); its .type is "normal" or "alternate", and xterm.js emits onBufferChange when it switches. [1][2]
Details worth noting
- Switching to the alternate buffer preserves the normal buffer’s contents so they can be restored when you switch back. [3][4]
- Control sequences (e.g. CSI ? 47 h / CSI ? 47 l and related 1047/1049 variants) are what apps send to toggle these modes; xterm.js implements these modes and reports them in its VT features. [3][4]
Sources
- xterm.js API: IBufferNamespace / active, normal, alternate and onBufferChange. [1]
- xterm.js IBuffer (properties including .type). [2]
- Terminal/alternate screen description (ALTBUF behavior). [3]
- xterm.js buffer system / BufferSet (normal vs alternate behavior). [4]
🌐 Web query:
xterm.js getSelectionPosition() type definition inclusive exclusive
💡 Result:
getSelectionPosition() returns an IBufferRange { start: IBufferCellPosition, end: IBufferCellPosition } (positions are 1‑based). The range’s start and end are inclusive (i.e. both endpoints count). See the buffer-range length calculation which uses (end.x - start.x + 1). [1][2]
Note: some APIs that take column arguments (e.g. IBufferLine.translateToString) document startColumn as inclusive and endColumn as exclusive. [3]
Sources:
- getRangeLength implementation (shows +1, meaning end is inclusive). [1]
- IBufferRange / IBufferCellPosition type docs. [2]
- IBufferLine.translateToString docs (start inclusive, end exclusive). [3]
Add alternate buffer guard and normalize selection direction; fix selection length calculation for inclusive end position.
Two bugs need fixing:
-
Selection direction: if user drags right-to-left, xterm may report
start.x > end.x, makingselectionLengthnegative. The currentif (selectionLength <= 0) return false;silently skips the operation instead of handling it. Normalize withMath.min/max. -
Alternate buffer: this handler runs on
xterm.buffer.activewithout checking if it's fullscreen-mode (vim, less, etc.). Add guardif (xterm.buffer.active !== xterm.buffer.normal) return false;to prevent interference with alternate screen apps. -
Selection length is off by one:
getSelectionPosition()returns inclusive endpoints (verified in xterm.js getRangeLength which usesend.x - start.x + 1). Current code treatsend.xas exclusive, undercounting by 1. Fix: changeselectionLength = selEndX - selStartX;toselectionLength = selEndX - selStartX + 1;
function handleSelectionDelete(
xterm: XTerm,
event: KeyboardEvent,
onWrite?: (data: string) => void,
): boolean {
if (!onWrite) return false;
+ if (xterm.buffer.active !== xterm.buffer.normal) return false;
// Only handle keydown events
if (event.type !== "keydown") return false;
@@
- const selStartX = selectionPosition.start.x;
- const selEndX = selectionPosition.end.x;
- const selectionLength = selEndX - selStartX;
+ const selStartX = Math.min(selectionPosition.start.x, selectionPosition.end.x);
+ const selEndX = Math.max(selectionPosition.start.x, selectionPosition.end.x);
+ const selectionLength = selEndX - selStartX + 1;🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
around lines 230 to 306, normalize selection direction, guard against the
alternate buffer, and correct inclusive selection length: ensure you
early-return if xterm.buffer.active !== xterm.buffer.normal to avoid interfering
with alternate-screen apps, compute selStartX = Math.min(start.x, end.x) and
selEndX = Math.max(start.x, end.x) to handle right-to-left selections, and
compute selectionLength = selEndX - selStartX + 1 (instead of the current
exclusive calculation); keep existing checks for single-character keydown and
modifiers, then proceed to preventDefault, clearSelection and onWrite as before.
…e cursor Remove the selection delete on type feature as it wasn't working correctly. Keep only the click-to-move cursor functionality. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…to-move - Clamp click coordinates to valid terminal grid range [0..cols-1, 0..rows-1] to prevent excessive delta calculations from out-of-bounds clicks - Add alternate buffer check to bail out when in full-screen apps (vim, less, etc.) - Document private API dependency on _core._renderService.dimensions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.