implement more font and xterm rendering improvements#324
Conversation
Add Nerd Font families to terminal font stack to properly render Powerlevel10k and similar Zsh themes that use special glyphs. Closes #307 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@Kitenite has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 2 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 ignored due to path filters (1)
📒 Files selected for processing (9)
WalkthroughReworks the desktop terminal subsystem: adds GPU-accelerated renderer with fallbacks and ligature/image addons, adds DataBatcher for batched IPC of terminal output, reorganizes terminal modules into a new terminal/ namespace with session/manager/types, updates terminal env and shell wrapper (ZDOTDIR ordering), and adds xterm-related dependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant PTY as PtyProcess
participant Session as TerminalSession
participant DB as DataBatcher
participant Main as Main (IPC)
participant Renderer as Renderer/XTerm
PTY->>Session: data chunk (Buffer)
Session->>DB: dataBatcher.write(Buffer)
DB-->>DB: accumulate + handle UTF‑8 boundary
par Flush conditions
DB-->>DB: time-based flush (~16ms)
and
DB-->>DB: size-based flush (≥200KB)
end
DB->>Main: onFlush(decoded string)
Main->>Renderer: IPC send terminal data
Renderer->>Renderer: render frame (WebGL → Canvas fallback if needed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.ts (1)
6-18: Nice encapsulation of Nerd Font-aware font familyCentralizing the Nerd Font + monospace fallbacks into
TERMINAL_FONT_FAMILYand wiring it intoTERMINAL_OPTIONS.fontFamilykeeps the config readable and makes future tweaks to the font stack straightforward. The resulting joined string is valid forITerminalOptions["fontFamily"]and fits the renderer constraints (no Node imports).If you later need this font stack elsewhere (e.g., other terminal-like views), consider exporting
TERMINAL_FONT_FAMILYfrom this module instead of duplicating the list.Also applies to: 23-23
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
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/config.ts
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/config.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain type safety and avoid using
anyunless absolutely necessary in TypeScript code
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - only in src/main/
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.ts
🧠 Learnings (2)
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/terminal-*.ts : Use node-pty for terminal session management in the desktop app
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Please use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.ts
⏰ 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). (1)
- GitHub Check: Build
Add MesloLGM Nerd Font variants (recommended by Oh My Posh) and CaskaydiaCove Nerd Font (popular with Starship) to terminal font stack. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
The zsh wrapper was resetting ZDOTDIR AFTER sourcing the user's .zshrc, which caused Oh My Zsh to crash because it initializes paths based on ZDOTDIR during .zshrc sourcing. Moving the ZDOTDIR reset to happen BEFORE sourcing ensures Oh My Zsh sees the correct home directory. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/main/lib/agent-setup.ts (1)
144-156: LGTM! The ZDOTDIR reset timing fix is correct.The change ensures Oh My Zsh and similar frameworks see the correct ZDOTDIR during initialization by exporting it before sourcing the user's
.zshrc. This fixes the issue where frameworks would look for configuration in the wrong directory.Optional refinement: Distinguish between unset and set-to-HOME ZDOTDIR.
When ZDOTDIR was originally unset, the code sets
SUPERSET_ORIG_ZDOTDIRto the home directory (line 219), then exportsZDOTDIR="$HOME"in the wrapper (line 150). This is not strictly equivalent to leaving ZDOTDIR unset, though most frameworks treat them identically. Edge cases might exist where scripts check[ -n "$ZDOTDIR" ]vs checking its value.Consider using a separate flag to distinguish whether ZDOTDIR was originally set:
// In getShellEnv: return { SUPERSET_ORIG_ZDOTDIR: process.env.ZDOTDIR || os.homedir(), SUPERSET_ZDOTDIR_WAS_SET: process.env.ZDOTDIR ? "1" : "0", ZDOTDIR: ZSH_DIR, };Then in the wrapper, conditionally export or unset:
# Reset ZDOTDIR before sourcing user config so Oh My Zsh sees correct paths -export ZDOTDIR="$_superset_home" +if [ "$SUPERSET_ZDOTDIR_WAS_SET" = "1" ]; then + export ZDOTDIR="$_superset_home" +else + unset ZDOTDIR +fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/main/lib/agent-setup.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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/main/lib/agent-setup.ts
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/main/lib/agent-setup.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain type safety and avoid using
anyunless absolutely necessary in TypeScript code
Files:
apps/desktop/src/main/lib/agent-setup.ts
⏰ 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). (1)
- GitHub Check: Build
xterm.js improvements: - Add WebGL renderer with automatic Canvas fallback for GPU acceleration - Add ImageAddon for inline image support (iTerm2 protocol) - Add LigaturesAddon for font ligature support (async loaded) - Improve terminal options: 10k scrollback, macOptionIsMeta, bar cursor node-pty improvements: - Add DataBatcher for IPC performance (~60fps batching like Hyper) - Add enhanced env vars: TERM_PROGRAM, COLORTERM=truecolor, LANG Based on vercel/hyper terminal implementation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/desktop/src/main/lib/data-batcher.ts (1)
15-19: Batch size threshold uses “chars” but constant is “bytes” — consider byte-based accounting.
BATCH_MAX_SIZEis documented as 200KB, butthis.buffer.lengthis UTF-16 code units. If you want a true byte cap, considerBuffer.byteLength(this.buffer, "utf8")(or track bytes written separately).Also applies to: 36-45
apps/desktop/src/main/lib/terminal-manager.ts (2)
164-179: Environment defaults:LANGfallback may be surprising on some platforms; verify intent.Setting
LANGto"en_US.UTF-8"when missing is probably fine for UTF-8 safety, but please sanity-check on Windows/non-US locales (some tools behave differently whenLANGis forced).
279-286: Disposesession.dataBatcheron “already-dead” cleanup paths (defensive timer cleanup).Even if
onExitusually runs, it’d be safer tosession.dataBatcher.dispose()anywhere you delete/clear a session without going throughonExit(e.g.,cleanup()else-branch,killByWorkspaceIdelse-branch), to ensure any pending timer is cleared.} else { // Clean up history for already-dead sessions session.deleteHistoryOnExit = true; + session.dataBatcher.dispose(); await this.closeHistory(session); this.sessions.delete(paneId); results.push(Promise.resolve(true)); }Also applies to: 589-592, 522-528
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (1)
60-97: Harden WebGL context-loss fallback (avoid keeping disposed renderer reference).In
onContextLoss, consider settingrenderer = nullbefore/afterwebglAddon.dispose()sodispose()doesn’t keep pointing at the disposed WebGL addon if Canvas creation fails.webglAddon.onContextLoss(() => { console.warn("[Terminal] WebGL context lost, falling back to Canvas"); - webglAddon.dispose(); + renderer = null; + webglAddon.dispose(); try { renderer = new CanvasAddon(); xterm.loadAddon(renderer); } catch (canvasError) { console.error("[Terminal] Canvas fallback failed:", canvasError); } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
apps/desktop/package.json(1 hunks)apps/desktop/src/main/lib/data-batcher.ts(1 hunks)apps/desktop/src/main/lib/terminal-manager.ts(7 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
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/config.tsapps/desktop/src/main/lib/data-batcher.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/main/lib/terminal-manager.ts
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/config.tsapps/desktop/src/main/lib/data-batcher.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/main/lib/terminal-manager.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain type safety and avoid using
anyunless absolutely necessary in TypeScript code
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.tsapps/desktop/src/main/lib/data-batcher.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/main/lib/terminal-manager.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - only in src/main/
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
apps/desktop/src/main/lib/terminal-*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use node-pty for terminal session management in the desktop app
Files:
apps/desktop/src/main/lib/terminal-manager.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/terminal-*.ts : Use node-pty for terminal session management in the desktop app
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/terminal-*.ts : Use node-pty for terminal session management in the desktop app
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.tsapps/desktop/src/main/lib/data-batcher.tsapps/desktop/package.jsonapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/main/lib/terminal-manager.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables from monorepo root .env file in src/main/index.ts with override: true
Applied to files:
apps/desktop/src/main/lib/terminal-manager.ts
🧬 Code graph analysis (1)
apps/desktop/src/main/lib/terminal-manager.ts (1)
apps/desktop/src/main/lib/data-batcher.ts (1)
DataBatcher(20-84)
🔇 Additional comments (4)
apps/desktop/package.json (1)
45-55: Verify xterm addon ↔ xterm core version compatibility (and lockfile update).You added
@xterm/addon-canvasand@xterm/addon-ligatures; please confirm these versions are intended/supported with@xterm/xterm@^5.5.0, and ensure the lockfile was updated accordingly.apps/desktop/src/main/lib/terminal-manager.ts (1)
223-228: DataBatcher changes event chunking — verify renderer-side assumptions (query suppression / protocol handling).You now emit batched payloads instead of per-pty chunk. Please confirm anything consuming
data:${paneId}(subscriptions/terminal parsing) doesn’t rely on chunk boundaries and still behaves correctly with ~16ms coalescing.Also applies to: 257-264
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (2)
143-157: Cleanup/lifecycle: verifyxterm.dispose()is called somewhere; otherwise new addons can leak.
cleanup()now disposes the renderer, but not thexterminstance (nor the async ligatures addon). If callers don’t reliably callxterm.dispose(), consider doing it here (or document the ownership contract clearly).Also applies to: 185-192
128-142: Addon load order looks sensible; ImageAddon addition is fine.
| // Font family with Nerd Font support for Oh My Posh, Powerlevel10k, Starship, and similar themes. | ||
| // Falls back to standard monospace fonts if Nerd Fonts aren't installed. | ||
| const TERMINAL_FONT_FAMILY = [ | ||
| "MesloLGM Nerd Font", // Recommended by Oh My Posh (Medium line gap) | ||
| "MesloLGM NF", | ||
| "MesloLGS NF", // Recommended by Powerlevel10k (Small line gap) | ||
| "MesloLGS Nerd Font", | ||
| "Hack Nerd Font", | ||
| "FiraCode Nerd Font", | ||
| "JetBrainsMono Nerd Font", | ||
| "CaskaydiaCove Nerd Font", // Popular with Windows Terminal / Starship | ||
| "Menlo", | ||
| "Monaco", | ||
| '"Courier New"', | ||
| "monospace", | ||
| ].join(", "); | ||
|
|
There was a problem hiding this comment.
Quote font names with spaces in fontFamily (CSS parsing bug).
Several entries contain spaces but aren’t quoted, so the font-family list may not resolve to the intended fonts.
const TERMINAL_FONT_FAMILY = [
- "MesloLGM Nerd Font", // Recommended by Oh My Posh (Medium line gap)
+ '"MesloLGM Nerd Font"', // Recommended by Oh My Posh (Medium line gap)
"MesloLGM NF",
- "MesloLGS NF", // Recommended by Powerlevel10k (Small line gap)
- "MesloLGS Nerd Font",
- "Hack Nerd Font",
- "FiraCode Nerd Font",
- "JetBrainsMono Nerd Font",
- "CaskaydiaCove Nerd Font", // Popular with Windows Terminal / Starship
+ "MesloLGS NF", // Recommended by Powerlevel10k (Small line gap)
+ '"MesloLGS Nerd Font"',
+ '"Hack Nerd Font"',
+ '"FiraCode Nerd Font"',
+ '"JetBrainsMono Nerd Font"',
+ '"CaskaydiaCove Nerd Font"', // Popular with Windows Terminal / Starship
"Menlo",
"Monaco",
'"Courier New"',
"monospace",
].join(", ");Also applies to: 26-41
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.ts (1)
7-20: Font names with spaces need quotes for proper CSS parsing.This was flagged in a previous review. Font names containing spaces (e.g.,
"MesloLGM Nerd Font","Hack Nerd Font") must be wrapped in inner quotes to be correctly parsed as CSSfont-familyvalues. Without quotes, the browser may not resolve these fonts correctly.Apply this diff:
const TERMINAL_FONT_FAMILY = [ - "MesloLGM Nerd Font", + '"MesloLGM Nerd Font"', "MesloLGM NF", "MesloLGS NF", - "MesloLGS Nerd Font", - "Hack Nerd Font", - "FiraCode Nerd Font", - "JetBrainsMono Nerd Font", - "CaskaydiaCove Nerd Font", + '"MesloLGS Nerd Font"', + '"Hack Nerd Font"', + '"FiraCode Nerd Font"', + '"JetBrainsMono Nerd Font"', + '"CaskaydiaCove Nerd Font"', "Menlo", "Monaco", '"Courier New"', "monospace", ].join(", ");
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (1)
119-129: Consider ImageAddon configuration for memory management in long-running sessions.The
ImageAddonis loaded with default settings. For long-running terminal sessions with heavy image output, consider configuringstorageLimit(FIFO cache limit, default 128 MB),pixelLimit(per-image pixel limit, default 16777216), orsixelSizeLimit(SIXEL sequence size limit, default 25000000) to control memory usage and prevent excessive accumulation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/main/lib/agent-setup.ts(1 hunks)apps/desktop/src/main/lib/terminal-manager.ts(7 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/main/lib/agent-setup.ts
🧰 Additional context used
📓 Path-based instructions (5)
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/config.tsapps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
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/config.tsapps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain type safety and avoid using
anyunless absolutely necessary in TypeScript code
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.tsapps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - only in src/main/
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
apps/desktop/src/main/lib/terminal-*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use node-pty for terminal session management in the desktop app
Files:
apps/desktop/src/main/lib/terminal-manager.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/terminal-*.ts : Use node-pty for terminal session management in the desktop app
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/terminal-*.ts : Use node-pty for terminal session management in the desktop app
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.tsapps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
Applied to files:
apps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
🔇 Additional comments (7)
apps/desktop/src/main/lib/terminal-manager.ts (3)
164-167: LGTM! Good terminal environment setup.The new environment variables (
TERM_PROGRAM,COLORTERM,LANG) properly identify the terminal capabilities. One minor note:process.env.npm_package_versionis only available when running via npm scripts, so the fallback to"1.0.0"is appropriate for production builds.
219-223: DataBatcher integration looks correct.The batcher is properly scoped per session with the callback emitting to the correct pane channel. This should effectively reduce IPC overhead by batching terminal output at ~60fps.
275-276: No action required. TheDataBatcher.dispose()implementation properly flushes all remaining buffered data by callingflush(), which handles timeout cleanup, decoder finalization, and emission of all buffered content via theonFlushcallback before state reset.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.ts (1)
28-34: Good terminal option additions.The new options enhance the terminal UX appropriately:
scrollback: 10000is a reasonable buffer size,macOptionIsMeta: trueenables proper Meta key behavior for shell shortcuts, and the cursor/fast-scroll settings are sensible defaults.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (3)
55-89: Well-structured GPU renderer fallback logic.The WebGL → Canvas → default renderer fallback chain is correctly implemented. The
onContextLosshandler appropriately disposes the WebGL addon and attempts Canvas fallback. Empty catch blocks are acceptable here for graceful degradation.
131-139: Dynamic import for ligatures is a good pattern for optional features.Loading ligatures asynchronously prevents blocking terminal initialization if the addon is unavailable or the font doesn't support ligatures. The error swallowing is intentional for this optional enhancement.
170-173: Cleanup properly disposes the renderer.The cleanup function now correctly calls
renderer.dispose()alongside the query suppression cleanup, ensuring GPU resources are released when the terminal instance is destroyed.
Move decoder.end() from flush() to dispose() so the StringDecoder retains state for incomplete multi-byte UTF-8 sequences that may span multiple PTY data chunks. Before: decoder.end() was called every 16ms, corrupting emoji and other multi-byte characters split across reads. After: decoder maintains state across flushes, only finalized on stream termination. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Kitenite
changed the title
Security: - Remove Electron's GOOGLE_API_KEY from terminal environment Reliability: - Add shell fallback mechanism: if shell crashes within 1s, retry with /bin/sh - Use default-shell package for robust shell detection Compatibility: - Better locale detection: check LANG, LC_ALL, system locale, fallback to en_US.UTF-8 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Split the 760-line terminal-manager.ts into focused modules: - terminal/types.ts - Session interfaces and type definitions - terminal/env.ts - Environment setup, shell detection, locale - terminal/session.ts - Session creation, data handling, history - terminal/manager.ts - Main TerminalManager class (orchestration only) - terminal/index.ts - Public exports Benefits: - Each file has a single responsibility - Easier to test individual components - Better code navigation and maintainability - Clearer separation of concerns 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary by CodeRabbit
New Features
New Dependencies
Enhancements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.