Skip to content

Revert xterm.js v6.0.0 upgrade back to v5.5.0#1226

Merged
Kitenite merged 5 commits into
mainfrom
kitenite/revert-xterm-6-upgrade
Feb 5, 2026
Merged

Revert xterm.js v6.0.0 upgrade back to v5.5.0#1226
Kitenite merged 5 commits into
mainfrom
kitenite/revert-xterm-6-upgrade

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Feb 5, 2026

Summary

Test plan

  • Verify terminal renders correctly with xterm v5.5.0
  • Verify scroll-to-bottom button works
  • Verify terminal CSS/styling is correct
  • Verify terminal addons (fit, webgl, search, clipboard, etc.) load without errors

Summary by CodeRabbit

  • New Features

    • Canvas-based terminal rendering added (preferred on macOS), plus WebGL/DOM fallbacks.
    • Fast scroll using Alt key with adjustable sensitivity.
    • Scroll-to-bottom now supports smooth/instant scroll behavior.
  • Bug Fixes

    • More reliable viewport scroll handling for consistent scroll button visibility and behavior.
  • Chores

    • Updated terminal-related dependencies and refined terminal CSS for improved sizing and layout.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 5, 2026

📝 Walkthrough

Walkthrough

This PR changes the desktop app's xterm integration: it removes an explicit Vite alias for @xterm/headless, downgrades and stabilizes xterm/addon dependencies (adds canvas/web-links), adds Canvas renderer support and related fallbacks, switches scroll listening to the DOM viewport, and introduces fast-scroll config and small type additions.

Changes

Cohort / File(s) Summary
Vite config & dependencies
apps/desktop/electron.vite.config.ts, apps/desktop/package.json
Removed explicit alias for @xterm/headless; replaced beta xterm packages with stable ranges; added @xterm/addon-canvas and @xterm/addon-web-links.
Renderer logic & config
apps/desktop/src/renderer/.../Terminal/helpers.ts, apps/desktop/src/renderer/.../Terminal/config.ts
Added canvas to renderer kinds, prefer canvas on macOS, added fastScrollModifier and fastScrollSensitivity, updated WebGL<->Canvas fallback and disposal logic.
Scroll behavior & listeners
apps/desktop/src/renderer/.../Terminal/ScrollToBottomButton/ScrollToBottomButton.tsx, apps/desktop/src/renderer/.../Terminal/utils.ts
Replaced terminal.onScroll subscription with DOM viewport scroll listener; scrollToBottom now accepts an optional ScrollBehavior and uses viewport.scrollTo when available.
CSS sizing
apps/desktop/src/renderer/globals.css
Refactored xterm sizing rules to target .xterm and .xterm-screen explicitly and set .xterm-screen to width/height 100%.
Types for file tree
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/types.ts
Added OnFileOpen type and TreeActionResult interface; imported FileTreeNode type.

Sequence Diagram

sequenceDiagram
    participant App as Terminal Component
    participant Helper as Renderer Helper
    participant Canvas as Canvas Renderer
    participant WebGL as WebGL Renderer
    participant DOM as DOM Renderer
    participant Viewport as DOM Viewport

    App->>Helper: loadRenderer()
    alt Platform is macOS
        Helper->>Canvas: tryLoadCanvas()
        Canvas-->>Helper: success / failure
        alt Canvas fails
            Helper->>WebGL: loadWebGL()
            WebGL-->>Helper: loaded / failed
        end
    else Other Platform
        Helper->>WebGL: loadWebGL()
        WebGL-->>Helper: loaded / failed
        alt WebGL context lost
            Helper->>Canvas: attempt Canvas fallback
            Canvas-->>Helper: success / failure
            alt Canvas fails
                Helper->>DOM: fallback to DOM
                DOM-->>Helper: rendering active
            end
        end
    end
    Helper-->>App: update rendering kind

    App->>Viewport: attach scroll listener
    Viewport->>App: user scroll event
    App->>App: scrollToBottom(behavior)
    App->>Viewport: viewport.scrollTo({ top: scrollHeight, behavior })
    Viewport-->>App: scroll applied
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested reviewers

  • AviPeltz

"I hopped through code with whiskers bright,
Canvas, WebGL, and DOM now share the light,
Scrolls that glide and fallbacks that mend,
Tiny type gifts and deps to tend,
A rabbit cheers — the terminal's delight! 🐇✨"

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: reverting xterm.js from v6.0.0 back to v5.5.0, which aligns with the primary objective of the PR.
Description check ✅ Passed The description provides a clear summary of changes and test plan, but lacks required template sections like Related Issues, Type of Change, and Screenshots fields.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch kitenite/revert-xterm-6-upgrade

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 5, 2026

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch
  • ✅ Electric Fly.io app

Thank you for your contribution! 🎉

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (1)

88-156: ⚠️ Potential issue | 🟡 Minor

Update TerminalRenderer.kind to reflect actual renderer after context-loss fallback.

Currently, kind is reassigned within the webglAddon.onContextLoss() callback (lines 127, 133), but the returned object's kind property—captured at function return time—won't reflect these changes. If any code later reads rendererRef.current.kind (for logging, telemetry, or feature gating), it will report stale state even after a Canvas fallback.

Create a mutable rendererState object and introduce a setKind() helper to keep both the local variable and the returned object in sync:

Proposed fix
function loadRenderer(xterm: XTerm): TerminalRenderer {
	let renderer: WebglAddon | CanvasAddon | null = null;
	let webglAddon: WebglAddon | null = null;
	let kind: TerminalRenderer["kind"] = "dom";
+	const rendererState: TerminalRenderer = {
+		kind,
+		dispose: () => renderer?.dispose(),
+		clearTextureAtlas: undefined,
+	};
+	const setKind = (nextKind: TerminalRenderer["kind"]) => {
+		kind = nextKind;
+		rendererState.kind = nextKind;
+	};

	const preferred = getPreferredRenderer();

	if (preferred === "dom") {
		return { kind: "dom", dispose: () => {}, clearTextureAtlas: undefined };
	}

	const tryLoadCanvas = () => {
		try {
			renderer = new CanvasAddon();
			xterm.loadAddon(renderer);
-			kind = "canvas";
+			setKind("canvas");
		} catch {
			// Canvas fallback failed, use default renderer
		}
	};

	if (preferred === "canvas") {
		tryLoadCanvas();
-		return {
-			kind,
-			dispose: () => renderer?.dispose(),
-			clearTextureAtlas: undefined,
-		};
+		return rendererState;
	}

	try {
		webglAddon = new WebglAddon();

		webglAddon.onContextLoss(() => {
			webglAddon?.dispose();
			webglAddon = null;
			try {
				renderer = new CanvasAddon();
				xterm.loadAddon(renderer);
-				kind = "canvas";
+				setKind("canvas");
				// Force refresh after context loss recovery
				xterm.refresh(0, xterm.rows - 1);
			} catch {
				// Canvas fallback failed, use default renderer
				renderer = null;
-				kind = "dom";
+				setKind("dom");
			}
		});

		xterm.loadAddon(webglAddon);
		renderer = webglAddon;
-		kind = "webgl";
+		setKind("webgl");
	} catch {
		tryLoadCanvas();
	}

-	return {
-		kind,
-		dispose: () => renderer?.dispose(),
-		clearTextureAtlas: webglAddon
-			? () => {
-					try {
-						webglAddon?.clearTextureAtlas();
-					} catch (error) {
-						console.warn("[Terminal] WebGL clearTextureAtlas() failed:", error);
-					}
-				}
-			: undefined,
-	};
+	rendererState.clearTextureAtlas = webglAddon
+		? () => {
+				try {
+					webglAddon?.clearTextureAtlas();
+				} catch (error) {
+					console.warn("[Terminal] WebGL clearTextureAtlas() failed:", error);
+				}
+			}
+		: undefined;
+	return rendererState;
}
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.ts`:
- Around line 45-46: The hardcoded xterm option values fastScrollModifier and
fastScrollSensitivity should be pulled into named module-level constants for
consistency; create constants (e.g., FAST_SCROLL_MODIFIER and
FAST_SCROLL_SENSITIVITY) at the top of the module and replace the inline values
in the config object (fastScrollModifier: "alt" and fastScrollSensitivity: 5)
with those constants, keeping the same values and exporting or keeping scope
identical to existing constants pattern in the file.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts`:
- Around line 99-107: The catch blocks in tryLoadCanvas (and the other
renderer-loading blocks around lines referenced) silently swallow errors; update
those catches to log the error with a consistent prefixed context (e.g.
"[terminal/renderer]") including the thrown error message and stack, and if a
partially constructed addon instance (renderer or similar) exists call its
dispose() (or appropriate cleanup) before continuing; specifically modify the
tryLoadCanvas flow that creates CanvasAddon and calls xterm.loadAddon(renderer)
to catch (err) -> console.error("[terminal/renderer] CanvasAddon load failed:",
err) and dispose renderer if set, and apply the same pattern to the other
renderer load attempts that currently have empty catches.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/utils.ts`:
- Around line 8-20: The function scrollToBottom currently uses a non-standard
ScrollBehavior value "instant" and positional params; change its signature to
accept an options object (e.g., scrollToBottom({ terminal, behavior = "auto" }:
{ terminal: Terminal; behavior?: ScrollBehavior })) and replace the default
"instant" with the standard "auto" (or allow "smooth") so typing matches
lib.dom; keep the existing logic (querySelector(".xterm-viewport") then
viewport.scrollTo({...}) or terminal.scrollToBottom() fallback) but pass the
behavior through the options object, and update all callers from
scrollToBottom(xterm) to scrollToBottom({ terminal: xterm }).

Comment on lines +45 to +46
fastScrollModifier: "alt",
fastScrollSensitivity: 5,
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

🧩 Analysis chain

🏁 Script executed:

cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.ts

Repository: superset-sh/superset

Length of output: 1933


🏁 Script executed:

# Search for package.json to verify xterm version
fd package.json --maxdepth 3 | head -5 | xargs grep -l "@xterm/xterm" 2>/dev/null

Repository: superset-sh/superset

Length of output: 89


🏁 Script executed:

# Check if fastScrollModifier and fastScrollSensitivity are documented in xterm types
# First, let's see what's in node_modules or type definitions
fd "xterm" -t d --maxdepth 3 | grep -E "(node_modules|types)" | head -5

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

cat -n apps/desktop/package.json | grep -A 5 -B 5 "@xterm/xterm"

Repository: superset-sh/superset

Length of output: 473


🏁 Script executed:

# Let's check if node_modules or bun.lockb exists and search for xterm types
fd bun.lockb package-lock.json yarn.lock -t f | head -1

Repository: superset-sh/superset

Length of output: 224


🏁 Script executed:

# Search for any xterm-related type definitions or version info in the codebase
rg "fastScrollModifier|fastScrollSensitivity" --type ts --type tsx

Repository: superset-sh/superset

Length of output: 91


🏁 Script executed:

rg "fastScrollModifier|fastScrollSensitivity" -g "*.ts" -g "*.tsx"

Repository: superset-sh/superset

Length of output: 332


🌐 Web query:

@xterm/xterm 5.5.0 fastScrollModifier fastScrollSensitivity ITerminalOptions API

💡 Result:

In @xterm/xterm 5.5.0, these are ITerminalOptions fields:

interface ITerminalOptions {
  fastScrollModifier?: 'none' | 'alt' | 'ctrl' | 'shift';
  fastScrollSensitivity?: number;
}
  • fastScrollModifier: which modifier key must be held to enable “fast scrolling” (multiplied scroll speed). In v5+, use 'none' (not undefined) to disable the modifier requirement. [2] [3]
  • fastScrollSensitivity: the scroll-speed multiplier used during fast scrolling (when the modifier is held). [1] [3]

Sources: xterm.js API docs for ITerminalOptions [1], xterm.js v5.5.0 release notes (and v5 API note about 'none') [2], jsdocs.io API listing for the option types [3].


Extract fastScrollModifier and fastScrollSensitivity to named constants.

These xterm 5.5.0 options are valid and working, but should follow the file's existing pattern of extracting hardcoded values to constants at the module top for consistency and maintainability.

Suggested constant extraction
+const FAST_SCROLL_MODIFIER = "alt";
+const FAST_SCROLL_SENSITIVITY = 5;
+
 export const TERMINAL_OPTIONS: ITerminalOptions = {
 	cursorBlink: true,
 	fontSize: 14,
 	fontFamily: TERMINAL_FONT_FAMILY,
 	theme: TERMINAL_THEME,
 	allowProposedApi: true,
 	scrollback: 10000,
 	// Allow Option+key to type special characters on international keyboards (e.g., Option+2 = @)
 	macOptionIsMeta: false,
 	cursorStyle: "block",
 	cursorInactiveStyle: "outline",
-	fastScrollModifier: "alt",
-	fastScrollSensitivity: 5,
+	fastScrollModifier: FAST_SCROLL_MODIFIER,
+	fastScrollSensitivity: FAST_SCROLL_SENSITIVITY,
 	screenReaderMode: false,
 };
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.ts`
around lines 45 - 46, The hardcoded xterm option values fastScrollModifier and
fastScrollSensitivity should be pulled into named module-level constants for
consistency; create constants (e.g., FAST_SCROLL_MODIFIER and
FAST_SCROLL_SENSITIVITY) at the top of the module and replace the inline values
in the config object (fastScrollModifier: "alt" and fastScrollSensitivity: 5)
with those constants, keeping the same values and exporting or keeping scope
identical to existing constants pattern in the file.

Comment on lines +99 to +107
const tryLoadCanvas = () => {
try {
renderer = new CanvasAddon();
xterm.loadAddon(renderer);
kind = "canvas";
} catch {
// Canvas fallback failed, use default renderer
}
};
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

Log renderer load failures instead of silently swallowing them.

Right now the new catch blocks drop errors, making GPU renderer failures hard to diagnose. Log with a prefixed context and dispose partially created addons.

Proposed fix
 const tryLoadCanvas = () => {
 	try {
 		renderer = new CanvasAddon();
 		xterm.loadAddon(renderer);
 		kind = "canvas";
-	} catch {
-		// Canvas fallback failed, use default renderer
+	} catch (error) {
+		console.warn("[Terminal/renderer] Canvas addon failed to load:", error);
+		renderer?.dispose();
+		renderer = null;
 	}
 };
@@
 		webglAddon.onContextLoss(() => {
 			webglAddon?.dispose();
 			webglAddon = null;
 			try {
 				renderer = new CanvasAddon();
 				xterm.loadAddon(renderer);
 				kind = "canvas";
 				// Force refresh after context loss recovery
 				xterm.refresh(0, xterm.rows - 1);
-			} catch {
-				// Canvas fallback failed, use default renderer
-				renderer = null;
-				kind = "dom";
+			} catch (error) {
+				console.warn(
+					"[Terminal/renderer] Canvas fallback after WebGL context loss failed:",
+					error,
+				);
+				renderer?.dispose();
+				renderer = null;
+				kind = "dom";
 			}
 		});
@@
-	} catch {
-		tryLoadCanvas();
+	} catch (error) {
+		console.warn("[Terminal/renderer] WebGL addon failed to load:", error);
+		webglAddon?.dispose();
+		webglAddon = null;
+		tryLoadCanvas();
 	}
As per coding guidelines: Never swallow errors silently; at minimum log errors with context before rethrowing or handling them explicitly; Use prefixed console logging with consistent context pattern: [domain/operation] message for entry/exit of significant operations, external API calls, and error conditions.

Also applies to: 124-134, 140-142

🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts`
around lines 99 - 107, The catch blocks in tryLoadCanvas (and the other
renderer-loading blocks around lines referenced) silently swallow errors; update
those catches to log the error with a consistent prefixed context (e.g.
"[terminal/renderer]") including the thrown error message and stack, and if a
partially constructed addon instance (renderer or similar) exists call its
dispose() (or appropriate cleanup) before continuing; specifically modify the
tryLoadCanvas flow that creates CanvasAddon and calls xterm.loadAddon(renderer)
to catch (err) -> console.error("[terminal/renderer] CanvasAddon load failed:",
err) and dispose renderer if set, and apply the same pattern to the other
renderer load attempts that currently have empty catches.

Comment on lines +8 to +20
export function scrollToBottom(
terminal: Terminal,
behavior: ScrollBehavior = "instant",
): void {
const viewport = terminal.element?.querySelector(".xterm-viewport");
if (viewport) {
viewport.scrollTo({
top: viewport.scrollHeight,
behavior,
});
} else {
terminal.scrollToBottom();
}
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

🧩 Analysis chain

🏁 Script executed:

# Check if scrollToBottom function exists and find its call sites
rg "scrollToBottom" --type ts --type tsx -A 2 -B 2

Repository: superset-sh/superset

Length of output: 91


🏁 Script executed:

# Get the actual file to see the current implementation
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/utils.ts

Repository: superset-sh/superset

Length of output: 684


🏁 Script executed:

# Check TypeScript version and lib.dom definitions for ScrollBehavior
cat -n tsconfig.json | head -50

Repository: superset-sh/superset

Length of output: 109


🏁 Script executed:

# Find scrollToBottom call sites (without tsx filter to avoid error)
rg "scrollToBottom" --type ts -A 2 -B 2

Repository: superset-sh/superset

Length of output: 17260


🏁 Script executed:

# Find tsconfig.json
fd tsconfig.json

Repository: superset-sh/superset

Length of output: 578


🏁 Script executed:

# Check lib.dom type definitions for ScrollBehavior
python3 << 'EOF'
import json

# Check TypeScript lib.dom definitions for ScrollBehavior
# This is the standard definition in TypeScript
scroll_behavior_def = {
    "auto": "Default, equivalent to smooth",
    "smooth": "Scroll smoothly",
    "instant": "Scroll instantly"
}

# Checking DOM spec
print("Standard DOM ScrollBehavior values:")
print("From ECMAScript spec and MDN:")
print("- 'auto': instant scroll (default)")
print("- 'smooth': smooth scroll animation")
print("\nNOTE: 'instant' is NOT in the standard ScrollBehavior enum")
print("It was added in some proposals but is NOT in lib.dom.d.ts")
EOF

Repository: superset-sh/superset

Length of output: 318


🏁 Script executed:

# Check the actual TypeScript definitions available in the project
cat -n apps/desktop/tsconfig.json | head -100

Repository: superset-sh/superset

Length of output: 608


🏁 Script executed:

# Check the extended TypeScript config
cat -n packages/typescript/electron.json

Repository: superset-sh/superset

Length of output: 129


🏁 Script executed:

# Search for any ScrollBehavior type definitions or extensions in the codebase
rg "ScrollBehavior" --type ts -A 3 -B 3

Repository: superset-sh/superset

Length of output: 995


🏁 Script executed:

# Find the extended TypeScript config
fd "electron.json"

Repository: superset-sh/superset

Length of output: 96


🏁 Script executed:

# Check if there are any custom type definitions for ScrollBehavior
rg "declare.*ScrollBehavior" --type ts

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Try to compile and check if "instant" actually causes a type error
# by checking the node_modules for typescript definitions
find . -path ./node_modules -prune -o -name "lib.dom.d.ts" -type f -print

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Check if TypeScript compilation would fail with the current code
# by looking at build/lint configuration
cat -n apps/desktop/package.json | head -50

Repository: superset-sh/superset

Length of output: 2370


🏁 Script executed:

# Check the extended TypeScript config
cat -n tooling/typescript/electron.json

Repository: superset-sh/superset

Length of output: 524


🏁 Script executed:

# Check if there's a lib.dom override or custom types in the project
rg "instant" --type ts --type json | head -20

Repository: superset-sh/superset

Length of output: 1144


🏁 Script executed:

# Run the actual TypeScript type check to see if "instant" causes an error
cd apps/desktop && bun run typecheck 2>&1 | head -50

Repository: superset-sh/superset

Length of output: 105


🏁 Script executed:

# Check if the file is actually included in typecheck by looking for any type errors
rg "ScrollBehavior" tooling/typescript/ -A 2 -B 2

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Let's verify by looking at the base TypeScript config to understand the full setup
cat -n tooling/typescript/base.json | head -50

Repository: superset-sh/superset

Length of output: 867


🏁 Script executed:

# Check if ScrollBehavior is defined anywhere in the codebase (custom type)
rg "type ScrollBehavior|interface ScrollBehavior" --type ts

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Look at the actual imports in utils.ts to see where ScrollBehavior comes from
head -5 apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/utils.ts

Repository: superset-sh/superset

Length of output: 229


🏁 Script executed:

# Check if there's any type augmentation or global ScrollBehavior override
rg "ScrollBehavior\s*=" --type ts

Repository: superset-sh/superset

Length of output: 208


Replace non-standard "instant" with standard ScrollBehavior value and use options object per guidelines.

ScrollBehavior from lib.dom only supports "auto" or "smooth", not "instant"—this causes a type error. Additionally, this function has 2 parameters and should use an options object per the coding guidelines.

All 7 call sites currently pass only the terminal argument, making the refactoring straightforward:

♻️ Suggested refactor
-export function scrollToBottom(
-	terminal: Terminal,
-	behavior: ScrollBehavior = "instant",
-): void {
+type ScrollToBottomOptions = {
+	terminal: Terminal;
+	behavior?: ScrollBehavior;
+};
+
+export function scrollToBottom({
+	terminal,
+	behavior = "auto",
+}: ScrollToBottomOptions): void {
 	const viewport = terminal.element?.querySelector(".xterm-viewport");
 	if (viewport) {
 		viewport.scrollTo({
 			top: viewport.scrollHeight,
 			behavior,
 		});
 	} else {
 		terminal.scrollToBottom();
 	}
 }

Update call sites from scrollToBottom(xterm) to scrollToBottom({ terminal: xterm }) (or use alias { terminal } if xterm is already the variable name).

📝 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
export function scrollToBottom(
terminal: Terminal,
behavior: ScrollBehavior = "instant",
): void {
const viewport = terminal.element?.querySelector(".xterm-viewport");
if (viewport) {
viewport.scrollTo({
top: viewport.scrollHeight,
behavior,
});
} else {
terminal.scrollToBottom();
}
type ScrollToBottomOptions = {
terminal: Terminal;
behavior?: ScrollBehavior;
};
export function scrollToBottom({
terminal,
behavior = "auto",
}: ScrollToBottomOptions): void {
const viewport = terminal.element?.querySelector(".xterm-viewport");
if (viewport) {
viewport.scrollTo({
top: viewport.scrollHeight,
behavior,
});
} else {
terminal.scrollToBottom();
}
}
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/utils.ts`
around lines 8 - 20, The function scrollToBottom currently uses a non-standard
ScrollBehavior value "instant" and positional params; change its signature to
accept an options object (e.g., scrollToBottom({ terminal, behavior = "auto" }:
{ terminal: Terminal; behavior?: ScrollBehavior })) and replace the default
"instant" with the standard "auto" (or allow "smooth") so typing matches
lib.dom; keep the existing logic (querySelector(".xterm-viewport") then
viewport.scrollTo({...}) or terminal.scrollToBottom() fallback) but pass the
behavior through the options object, and update all callers from
scrollToBottom(xterm) to scrollToBottom({ terminal: xterm }).

…-6-upgrade

# Conflicts:
#	apps/desktop/package.json
#	bun.lock
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts`:
- Around line 62-69: Update the top doc comment to accurately describe the
actual renderer selection flow: note that the code prefers Canvas when the
preferred renderer is "canvas" (and on macOS), so WebGL is not attempted in that
case; otherwise it will try WebGL first and fall back to Canvas if WebGL fails.
Reference the TerminalRenderer type (kind: "webgl" | "canvas" | "dom") when
clarifying the ordering and the platform/preference exceptions so the comment
matches the implemented logic.

Comment on lines 62 to 69
/**
* Load GPU-accelerated renderer with automatic fallback.
* Tries WebGL first, falls back to DOM if WebGL fails.
* This follows VS Code's approach: WebGL → DOM (canvas addon removed in xterm.js 6.0).
* Tries WebGL first, falls back to Canvas if WebGL fails.
*/
export type TerminalRenderer = {
kind: "webgl" | "dom";
kind: "webgl" | "canvas" | "dom";
dispose: () => void;
clearTextureAtlas?: () => void;
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

Adjust the comment to reflect the canvas-preferred path.
Right now the doc says “WebGL first,” but when the preferred renderer is "canvas" (or on macOS), WebGL isn’t attempted. Consider updating the wording to match the actual flow.

✏️ Suggested comment tweak
- * Tries WebGL first, falls back to Canvas if WebGL fails.
+ * Tries the preferred renderer first (WebGL by default, Canvas on macOS),
+ * falling back to Canvas or DOM as needed.
📝 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
/**
* Load GPU-accelerated renderer with automatic fallback.
* Tries WebGL first, falls back to DOM if WebGL fails.
* This follows VS Code's approach: WebGL → DOM (canvas addon removed in xterm.js 6.0).
* Tries WebGL first, falls back to Canvas if WebGL fails.
*/
export type TerminalRenderer = {
kind: "webgl" | "dom";
kind: "webgl" | "canvas" | "dom";
dispose: () => void;
clearTextureAtlas?: () => void;
/**
* Load GPU-accelerated renderer with automatic fallback.
* Tries the preferred renderer first (WebGL by default, Canvas on macOS),
* falling back to Canvas or DOM as needed.
*/
export type TerminalRenderer = {
kind: "webgl" | "canvas" | "dom";
dispose: () => void;
clearTextureAtlas?: () => void;
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts`
around lines 62 - 69, Update the top doc comment to accurately describe the
actual renderer selection flow: note that the code prefers Canvas when the
preferred renderer is "canvas" (and on macOS), so WebGL is not attempted in that
case; otherwise it will try WebGL first and fall back to Canvas if WebGL fails.
Reference the TerminalRenderer type (kind: "webgl" | "canvas" | "dom") when
clarifying the ordering and the platform/preference exceptions so the comment
matches the implemented logic.

@Kitenite Kitenite merged commit f58e64f into main Feb 5, 2026
13 checks passed
@Kitenite Kitenite deleted the kitenite/revert-xterm-6-upgrade branch February 5, 2026 21:33
Kitenite added a commit that referenced this pull request Feb 6, 2026
Kitenite added a commit that referenced this pull request Feb 6, 2026
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.

1 participant