-
Notifications
You must be signed in to change notification settings - Fork 909
feat(desktop): add userTitle field for user-renamed tabs #289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
28915fe
72d71e6
06d2af5
c725109
c5dbfc3
4bae7dc
33a5398
043a42d
da5282e
e52395c
3178f52
af59e96
ed8cbe3
2ade020
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,12 +2,14 @@ import "@xterm/xterm/css/xterm.css"; | |
| import type { FitAddon } from "@xterm/addon-fit"; | ||
| import type { SearchAddon } from "@xterm/addon-search"; | ||
| import type { Terminal as XTerm } from "@xterm/xterm"; | ||
| import debounce from "lodash/debounce"; | ||
| import { useEffect, useRef, useState } from "react"; | ||
| import { useHotkeys } from "react-hotkeys-hook"; | ||
| import { trpc } from "renderer/lib/trpc"; | ||
| import { useTabsStore } from "renderer/stores/tabs/store"; | ||
| import { useTerminalTheme } from "renderer/stores/theme"; | ||
| import { HOTKEYS } from "shared/hotkeys"; | ||
| import { sanitizeForTitle } from "./commandBuffer"; | ||
| import { | ||
| createTerminalInstance, | ||
| getDefaultTerminalBg, | ||
|
|
@@ -26,22 +28,25 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { | |
| const panes = useTabsStore((s) => s.panes); | ||
| const pane = panes[paneId]; | ||
| const paneName = pane?.name || "Terminal"; | ||
| const parentTabId = pane?.tabId; | ||
| const terminalRef = useRef<HTMLDivElement>(null); | ||
| const xtermRef = useRef<XTerm | null>(null); | ||
| const fitAddonRef = useRef<FitAddon | null>(null); | ||
| const searchAddonRef = useRef<SearchAddon | null>(null); | ||
| const isExitedRef = useRef(false); | ||
| const pendingEventsRef = useRef<TerminalStreamEvent[]>([]); | ||
| const commandBufferRef = useRef(""); | ||
| const [subscriptionEnabled, setSubscriptionEnabled] = useState(false); | ||
| const [isSearchOpen, setIsSearchOpen] = useState(false); | ||
| const setFocusedPane = useTabsStore((s) => s.setFocusedPane); | ||
| const setTabAutoTitle = useTabsStore((s) => s.setTabAutoTitle); | ||
| const focusedPaneIds = useTabsStore((s) => s.focusedPaneIds); | ||
| const terminalTheme = useTerminalTheme(); | ||
|
|
||
| // Check if this terminal is the focused pane in its tab | ||
| const isFocused = pane?.tabId ? focusedPaneIds[pane.tabId] === paneId : false; | ||
| // Ref for initial theme to avoid recreating terminal on theme change | ||
| const initialThemeRef = useRef(terminalTheme); | ||
|
|
||
| // Ref to track focus state for use in terminal creation effect | ||
| const isFocused = pane?.tabId ? focusedPaneIds[pane.tabId] === paneId : false; | ||
| const isFocusedRef = useRef(isFocused); | ||
| isFocusedRef.current = isFocused; | ||
|
|
||
|
|
@@ -65,6 +70,21 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { | |
| resizeRef.current = resizeMutation.mutate; | ||
| detachRef.current = detachMutation.mutate; | ||
|
|
||
| const parentTabIdRef = useRef(parentTabId); | ||
| parentTabIdRef.current = parentTabId; | ||
|
|
||
| const paneNameRef = useRef(paneName); | ||
| paneNameRef.current = paneName; | ||
|
|
||
| const setTabAutoTitleRef = useRef(setTabAutoTitle); | ||
| setTabAutoTitleRef.current = setTabAutoTitle; | ||
|
|
||
| const debouncedSetTabAutoTitleRef = useRef( | ||
| debounce((tabId: string, title: string) => { | ||
| setTabAutoTitleRef.current(tabId, title); | ||
| }, 100), | ||
| ); | ||
|
|
||
| const handleStreamData = (event: TerminalStreamEvent) => { | ||
| if (!xtermRef.current) { | ||
| // Prevent data loss during terminal initialization | ||
|
|
@@ -90,14 +110,12 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { | |
| } | ||
| }; | ||
|
|
||
| // Use paneId (tabId) for stream subscription | ||
| trpc.terminal.stream.useSubscription(paneId, { | ||
| onData: handleStreamData, | ||
| // Always listen to prevent missing events during initialization | ||
| enabled: true, | ||
| }); | ||
|
|
||
| // Handler to set focused pane when terminal gains focus | ||
| // Use ref to avoid triggering full terminal recreation when focus handler changes | ||
| const handleTerminalFocusRef = useRef(() => {}); | ||
| handleTerminalFocusRef.current = () => { | ||
|
|
@@ -106,21 +124,18 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { | |
| } | ||
| }; | ||
|
|
||
| // Auto-close search when terminal loses focus | ||
| useEffect(() => { | ||
| if (!isFocused) { | ||
| setIsSearchOpen(false); | ||
| } | ||
| }, [isFocused]); | ||
|
|
||
| // Autofocus terminal when it becomes the focused pane (e.g., after split) | ||
| useEffect(() => { | ||
| if (isFocused && xtermRef.current) { | ||
| xtermRef.current.focus(); | ||
| } | ||
| }, [isFocused]); | ||
|
|
||
| // Toggle search with Cmd+F (only for the focused terminal) | ||
| useHotkeys( | ||
| HOTKEYS.FIND_IN_TERMINAL.keys, | ||
| () => { | ||
|
|
@@ -140,17 +155,19 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { | |
| xterm, | ||
| fitAddon, | ||
| cleanup: cleanupQuerySuppression, | ||
| } = createTerminalInstance(container, workspaceCwd, terminalTheme); | ||
| } = createTerminalInstance( | ||
| container, | ||
| workspaceCwd, | ||
| initialThemeRef.current, | ||
| ); | ||
| xtermRef.current = xterm; | ||
| fitAddonRef.current = fitAddon; | ||
| isExitedRef.current = false; | ||
|
|
||
| // Autofocus on initial render if this terminal is the focused pane | ||
| if (isFocusedRef.current) { | ||
| xterm.focus(); | ||
| } | ||
|
|
||
| // Load search addon for Cmd+F functionality | ||
| import("@xterm/addon-search").then(({ SearchAddon }) => { | ||
| if (isUnmounted) return; | ||
| const searchAddon = new SearchAddon(); | ||
|
|
@@ -194,7 +211,7 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { | |
| { | ||
| tabId: paneId, | ||
| workspaceId, | ||
| tabTitle: paneName, | ||
| tabTitle: paneNameRef.current, | ||
| cols: xterm.cols, | ||
| rows: xterm.rows, | ||
| }, | ||
|
|
@@ -214,16 +231,40 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { | |
| const handleTerminalInput = (data: string) => { | ||
| if (isExitedRef.current) { | ||
| restartTerminal(); | ||
| } else { | ||
| writeRef.current({ tabId: paneId, data }); | ||
| return; | ||
| } | ||
| writeRef.current({ tabId: paneId, data }); | ||
| }; | ||
|
|
||
| const handleKeyPress = (event: { | ||
| key: string; | ||
| domEvent: KeyboardEvent; | ||
| }) => { | ||
| const { domEvent } = event; | ||
| if (domEvent.key === "Enter") { | ||
| const title = sanitizeForTitle(commandBufferRef.current); | ||
| if (title && parentTabIdRef.current) { | ||
| debouncedSetTabAutoTitleRef.current(parentTabIdRef.current, title); | ||
| } | ||
| commandBufferRef.current = ""; | ||
| } else if (domEvent.key === "Backspace") { | ||
| commandBufferRef.current = commandBufferRef.current.slice(0, -1); | ||
| } else if (domEvent.key === "c" && domEvent.ctrlKey) { | ||
| commandBufferRef.current = ""; | ||
| } else if ( | ||
| domEvent.key.length === 1 && | ||
| !domEvent.ctrlKey && | ||
| !domEvent.metaKey | ||
| ) { | ||
| commandBufferRef.current += domEvent.key; | ||
| } | ||
| }; | ||
|
Comment on lines
+239
to
261
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Command buffering approach has significant limitations. The current implementation buffers keystrokes character-by-character, which works for simple sequential typing but fails in several common scenarios:
Given the PR objectives mention this is for deriving auto-titles (not comprehensive command tracking), this simplified approach may be acceptable for v1, but users will encounter incorrect titles in the above scenarios. Would you like me to:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's 2?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Kitenite Shell integration using OSC 133 sequences is a protocol that modern shells can emit to communicate with terminal emulators. Here's how it works: What are OSC 133 sequences? OSC (Operating System Command) 133 is a set of escape sequences that shells can output to mark semantic regions in the terminal. The key sequences are:
How this helps: Instead of trying to track keystrokes, the terminal would listen for these escape sequences. When the shell emits Example: Real-world implementations:
Trade-offs:
For your use case, shell integration would be significantly more robust but requires user setup. The current keystroke approach works immediately but with known limitations. Would you like me to sketch out what shell integration setup would look like, or are you leaning toward documenting the limitations of the current v1 approach?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. keystroke is fine, perhaps improve it with some simple cursor tracking
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Kitenite I'll add simple cursor tracking to handle arrow keys and character insertion at the cursor position. Here's an improved implementation: // Add a ref to track cursor position within the command buffer
const commandBufferRef = useRef("");
const cursorPositionRef = useRef(0);
// ... existing code ...
const handleKeyPress = (event: {
key: string;
domEvent: KeyboardEvent;
}) => {
const { domEvent } = event;
if (domEvent.key === "Enter") {
const title = sanitizeForTitle(commandBufferRef.current);
if (title && parentTabIdRef.current) {
debouncedSetTabAutoTitleRef.current(parentTabIdRef.current, title);
}
commandBufferRef.current = "";
cursorPositionRef.current = 0;
} else if (domEvent.key === "Backspace") {
// Delete character before cursor
if (cursorPositionRef.current > 0) {
const buffer = commandBufferRef.current;
commandBufferRef.current =
buffer.slice(0, cursorPositionRef.current - 1) +
buffer.slice(cursorPositionRef.current);
cursorPositionRef.current--;
}
} else if (domEvent.key === "ArrowLeft") {
// Move cursor left
cursorPositionRef.current = Math.max(0, cursorPositionRef.current - 1);
} else if (domEvent.key === "ArrowRight") {
// Move cursor right
cursorPositionRef.current = Math.min(
commandBufferRef.current.length,
cursorPositionRef.current + 1,
);
} else if (domEvent.key === "Home") {
// Move cursor to start
cursorPositionRef.current = 0;
} else if (domEvent.key === "End") {
// Move cursor to end
cursorPositionRef.current = commandBufferRef.current.length;
} else if (domEvent.key === "ArrowUp" || domEvent.key === "ArrowDown") {
// Shell history navigation - reset buffer
commandBufferRef.current = "";
cursorPositionRef.current = 0;
} else if (domEvent.key === "c" && domEvent.ctrlKey) {
commandBufferRef.current = "";
cursorPositionRef.current = 0;
} else if (
domEvent.key.length === 1 &&
!domEvent.ctrlKey &&
!domEvent.metaKey
) {
// Insert character at cursor position
const buffer = commandBufferRef.current;
commandBufferRef.current =
buffer.slice(0, cursorPositionRef.current) +
domEvent.key +
buffer.slice(cursorPositionRef.current);
cursorPositionRef.current++;
}
};What this adds:
Example scenario:
This handles basic cursor movement while keeping the implementation simple. Still won't track shell history contents or handle TUI apps, but covers the common editing cases. 🐰✏️ 🧠 Learnings used |
||
|
|
||
| createOrAttachRef.current( | ||
| { | ||
| tabId: paneId, | ||
| workspaceId, | ||
| tabTitle: paneName, | ||
| tabTitle: paneNameRef.current, | ||
| cols: xterm.cols, | ||
| rows: xterm.rows, | ||
| }, | ||
|
|
@@ -244,6 +285,7 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { | |
| ); | ||
|
|
||
| const inputDisposable = xterm.onData(handleTerminalInput); | ||
| const keyDisposable = xterm.onKey(handleKeyPress); | ||
|
|
||
| // Intercept keyboard events to handle app hotkeys and provide iTerm-like line continuation UX | ||
| const cleanupKeyboard = setupKeyboardHandler(xterm, { | ||
|
|
@@ -258,7 +300,6 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { | |
| }, | ||
| }); | ||
|
|
||
| // Setup focus listener to track focused pane (use ref to get latest handler) | ||
| const cleanupFocus = setupFocusListener(xterm, () => | ||
| handleTerminalFocusRef.current(), | ||
| ); | ||
|
|
@@ -271,26 +312,31 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { | |
| }, | ||
| ); | ||
| // Setup paste handler to ensure bracketed paste mode works for TUI apps like opencode | ||
| const cleanupPaste = setupPasteHandler(xterm); | ||
| const cleanupPaste = setupPasteHandler(xterm, { | ||
| onPaste: (text) => { | ||
| commandBufferRef.current += text; | ||
| }, | ||
| }); | ||
|
|
||
| return () => { | ||
| isUnmounted = true; | ||
| inputDisposable.dispose(); | ||
| keyDisposable.dispose(); | ||
| cleanupKeyboard(); | ||
| cleanupFocus?.(); | ||
| cleanupResize(); | ||
| cleanupPaste(); | ||
| cleanupQuerySuppression(); | ||
| // Keep PTY running for reattachment | ||
| debouncedSetTabAutoTitleRef.current?.cancel?.(); | ||
| // Detach instead of kill to keep PTY running for reattachment | ||
| detachRef.current({ tabId: paneId }); | ||
| setSubscriptionEnabled(false); | ||
| xterm.dispose(); | ||
| xtermRef.current = null; | ||
| searchAddonRef.current = null; | ||
| }; | ||
| }, [paneId, workspaceId, workspaceCwd, paneName, terminalTheme]); | ||
| }, [paneId, workspaceId, workspaceCwd]); | ||
|
|
||
| // Sync theme changes to xterm instance for live theme switching | ||
| useEffect(() => { | ||
| const xterm = xtermRef.current; | ||
| if (!xterm || !terminalTheme) return; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import { describe, expect, it } from "bun:test"; | ||
| import { sanitizeForTitle } from "./commandBuffer"; | ||
|
|
||
| describe("sanitizeForTitle", () => { | ||
| it("should keep normal text unchanged", () => { | ||
| expect(sanitizeForTitle("ls -la ./src")).toBe("ls -la ./src"); | ||
| }); | ||
|
|
||
| it("should keep uppercase letters", () => { | ||
| expect(sanitizeForTitle("openCode")).toBe("openCode"); | ||
| }); | ||
|
|
||
| it("should keep special characters", () => { | ||
| expect(sanitizeForTitle("npm install @scope/pkg")).toBe( | ||
| "npm install @scope/pkg", | ||
| ); | ||
| }); | ||
|
|
||
| it("should strip ANSI escape sequences", () => { | ||
| expect(sanitizeForTitle("\x1b[32mgreen\x1b[0m")).toBe("green"); | ||
| expect(sanitizeForTitle("\x1b[1;34mbold blue\x1b[0m")).toBe("bold blue"); | ||
| }); | ||
|
|
||
| it("should truncate to max length", () => { | ||
| const longCommand = "a".repeat(100); | ||
| const result = sanitizeForTitle(longCommand); | ||
| expect(result?.length).toBe(32); | ||
| }); | ||
|
|
||
| it("should return null for empty result", () => { | ||
| expect(sanitizeForTitle("")).toBeNull(); | ||
| }); | ||
|
|
||
| it("should return null for whitespace-only result", () => { | ||
| expect(sanitizeForTitle(" ")).toBeNull(); | ||
| }); | ||
|
|
||
| it("should trim whitespace", () => { | ||
| expect(sanitizeForTitle(" command ")).toBe("command"); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| import stripAnsi from "strip-ansi"; | ||
|
|
||
| const MAX_TITLE_LENGTH = 32; | ||
|
|
||
| export function sanitizeForTitle(text: string): string | null { | ||
| const cleaned = stripAnsi(text).trim().slice(0, MAX_TITLE_LENGTH); | ||
|
|
||
| return cleaned || null; | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.