Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,41 @@ export interface PasteHandlerOptions {
isBracketedPasteEnabled?: () => boolean;
}

/**
* Setup copy handler for xterm to trim trailing whitespace from copied text.
*
* Terminal emulators fill lines with whitespace to pad to the terminal width.
* When copying text, this results in unwanted trailing spaces on each line.
* This handler intercepts copy events and trims trailing whitespace from each
* line before writing to the clipboard.
*
* Returns a cleanup function to remove the handler.
*/
export function setupCopyHandler(xterm: XTerm): () => void {
const element = xterm.element;
if (!element) return () => {};

const handleCopy = (event: ClipboardEvent) => {
const selection = xterm.getSelection();
if (!selection) return;

// Trim trailing whitespace from each line while preserving intentional newlines
const trimmedText = selection
.split("\n")
.map((line) => line.trimEnd())
.join("\n");

event.preventDefault();
event.clipboardData?.setData("text/plain", trimmedText);
Comment on lines +340 to +341
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential silent copy failure if clipboardData is null.

When event.preventDefault() is called unconditionally on line 340, the default copy behavior is blocked. If clipboardData happens to be null (edge case in some browser/Electron contexts), line 341's optional chaining silently skips the write, leaving the user with an empty clipboard and no indication of failure.

Consider guarding the preventDefault() call or adding a fallback:

🛡️ Proposed fix
 	const handleCopy = (event: ClipboardEvent) => {
 		const selection = xterm.getSelection();
 		if (!selection) return;
+		if (!event.clipboardData) return;

 		// Trim trailing whitespace from each line while preserving intentional newlines
 		const trimmedText = selection
 			.split("\n")
 			.map((line) => line.trimEnd())
 			.join("\n");

 		event.preventDefault();
-		event.clipboardData?.setData("text/plain", trimmedText);
+		event.clipboardData.setData("text/plain", trimmedText);
 	};
📝 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
event.preventDefault();
event.clipboardData?.setData("text/plain", trimmedText);
const handleCopy = (event: ClipboardEvent) => {
const selection = xterm.getSelection();
if (!selection) return;
if (!event.clipboardData) return;
// Trim trailing whitespace from each line while preserving intentional newlines
const trimmedText = selection
.split("\n")
.map((line) => line.trimEnd())
.join("\n");
event.preventDefault();
event.clipboardData.setData("text/plain", trimmedText);
};
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts`
around lines 340 - 341, The current copy handler calls event.preventDefault()
unconditionally then uses event.clipboardData?.setData("text/plain",
trimmedText), which can silently fail if clipboardData is null; change the logic
in the copy handler so you only preventDefault when you have a successful path
to place text on the clipboard (e.g. check event.clipboardData exists before
calling event.preventDefault() and event.clipboardData.setData(...)), and add a
fallback that attempts navigator.clipboard.writeText(trimmedText) (with
try/catch) if event.clipboardData is absent; update references to
event.preventDefault, event.clipboardData?.setData and trimmedText in the
handler accordingly and surface/log errors if both methods fail.

};

element.addEventListener("copy", handleCopy);

return () => {
element.removeEventListener("copy", handleCopy);
};
}

/**
* Setup paste handler for xterm to ensure bracketed paste mode works correctly.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { DEBUG_TERMINAL, FIRST_RENDER_RESTORE_FALLBACK_MS } from "../config";
import {
createTerminalInstance,
setupClickToMoveCursor,
setupCopyHandler,
setupFocusListener,
setupKeyboardHandler,
setupPasteHandler,
Expand Down Expand Up @@ -518,6 +519,7 @@ export function useTerminalLifecycle({
onWrite: handleWrite,
isBracketedPasteEnabled: () => isBracketedPasteRef.current,
});
const cleanupCopy = setupCopyHandler(xterm);

const handleVisibilityChange = () => {
if (document.hidden || isUnmounted) return;
Expand Down Expand Up @@ -562,6 +564,7 @@ export function useTerminalLifecycle({
cleanupFocus?.();
cleanupResize();
cleanupPaste();
cleanupCopy();
cleanupQuerySuppression();
unregisterClearCallbackRef.current(paneId);
unregisterScrollToBottomCallbackRef.current(paneId);
Expand Down
Loading