Skip to content

Conversation

@bobicloudvision
Copy link

Summary

This pull request introduces a built-in terminal panel to the editor layout. It uses xterm.js with fit and webgl addons for rendering and node-pty for an interactive shell. The terminal automatically initializes when a project is open, follows the project’s working directory, resizes responsively, and restarts when the project path changes.

Changes Made

Terminal integration

  • Implemented EditorTerminal component rendering an embedded terminal panel with a sticky header.
  • Added xterm.js with FitAddon and optional WebglAddon for high-quality rendering and auto-fit behavior.
  • Wired node-pty via execNodePty to provide an interactive shell session.

Project-awareness and lifecycle

  • Terminal visibility is gated by projectConfiguration.path and updates via onProjectConfigurationChangedObservable.
  • Shell cwd is set relative to the current project path and re-initialized when the project changes.
  • Clean disposal of addons, terminal, and PTY on unmount; safe restart logic on project changes.

Responsiveness and UX

  • ResizeObserver triggers fit() for responsive sizing.
  • Terminal resizes on layout changes and forwards size to PTY.
  • Theming configured for editor look-and-feel with readable defaults and selection styling.

Benefits

  • Improves productivity by enabling command execution without leaving the editor.
  • Responsive and performant terminal rendering (WebGL fallback to canvas).
  • Project-aware shell sessions reduce friction (correct cwd, auto-restart on project switch).
  • Clean lifecycle management reduces leaks and ensures stability.

@julien-moreau
Copy link
Contributor

Hi @bobicloudvision !
Thanks a lot!! I'm reviewing right now

@julien-moreau
Copy link
Contributor

I tested it and this is great!!

2 major problems:

  • layout configuration is saved per user. That means you added a new tab that is not part of the already saved layout configuration for existing users. I propose to add a versionning for the layout that if the saved one doesn't match the current one then we use the default layout defined in .json file. If I do it for you, you can rebase on master?
  • The resize (fit addon) is not working properly (see screenshot below)
image

@julien-moreau
Copy link
Contributor

Also, I found only this version available for addon-webgl: "@xterm/addon-webgl": "0.18.0"

Copy link
Contributor

@julien-moreau julien-moreau left a comment

Choose a reason for hiding this comment

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

Just some few comments

"@radix-ui/react-tooltip": "^1.0.7",
"@radix-ui/react-radio-group": "^1.1.3",
"@xterm/addon-fit": "^0.10.0",
"@xterm/addon-webgl": "^0.19.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

I found only this version available for addon-webgl: "@xterm/addon-webgl": "0.18.0"

private _pty: NodePtyInstance | null = null;
private _projectPath: string | null = null;

constructor(props: IEditorTerminalProps) {
Copy link
Contributor

Choose a reason for hiding this comment

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

"public constructor"

// WebGL not available; keep default renderer
}

requestAnimationFrame(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use await waitNextFrame() that is available in src/tools/tools.ts

});

const ro = new ResizeObserver(() => {
requestAnimationFrame(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is next frame needed here? Just for my culture

name: "xterm-color",
encoding: "utf-8",
useConpty: false,
cwd: (options as any)?.cwd ?? process.cwd(),
Copy link
Contributor

Choose a reason for hiding this comment

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

options?.cwd ?? process.cwd() is enough

encoding: "utf-8",
useConpty: false,
cwd: (options as any)?.cwd ?? process.cwd(),
env: (options as any)?.env ?? process.env,
Copy link
Contributor

Choose a reason for hiding this comment

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

options?.env ?? process.env is enough

p.write(`${command}\n\r`);
}
const interactive: boolean = Boolean((options as any)?.interactive);
if (!interactive) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if (!options?.interactive) { ... } is enough

@julien-moreau
Copy link
Contributor

@bobicloudvision any news ?

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.

2 participants