From 3bea1f0db371a7c7e90a08725c442af3a75cdede Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 14 Jul 2021 14:41:40 -0700 Subject: [PATCH 1/2] Fallback to 80x30 dimensions in terminal This fixes an issue where when the terminal would be created without a container it initializes with 0x0 dimensions which gets set to the minimum of 2x1 instead of the expected default of 80x30 (which node-pty respects). This 80x30 is now hardcoded in VS Code. Fixes #128342 --- .../terminal/browser/terminalInstance.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index 7a68ad1cf54e8..e22c8d14b4266 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -79,7 +79,10 @@ const enum Constants { * terminal process. This period helps ensure the terminal has good initial dimensions to work * with if it's going to be a foreground terminal. */ - WaitForContainerThreshold = 100 + WaitForContainerThreshold = 100, + + DefaultCols = 80, + DefaultRows = 30, } let xtermConstructor: Promise | undefined; @@ -538,9 +541,8 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { const editorOptions = this._configurationService.getValue('editor'); const xterm = new Terminal({ - // TODO: Replace null with undefined when https://github.com/xtermjs/xterm.js/issues/3329 is resolved - cols: this._cols || null as any, - rows: this._rows || null as any, + cols: this._cols || Constants.DefaultCols, + rows: this._rows || Constants.DefaultRows, altClickMovesCursor: config.altClickMovesCursor && editorOptions.multiCursorModifier === 'alt', scrollback: config.scrollback, theme: this._getXtermTheme(), @@ -1159,8 +1161,15 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { if (this._isDisposed) { return; } + + // Re-evaluate dimensions if the container has been set since the xterm instance was created + if (this._container && this._cols === 0 && this._rows === 0) { + this._initDimensions(); + this._xterm?.resize(this._cols || Constants.DefaultCols, this._rows || Constants.DefaultRows); + } + const hadIcon = !!this.shellLaunchConfig.icon; - await this._processManager.createProcess(this._shellLaunchConfig, this._cols, this._rows, this._accessibilityService.isScreenReaderOptimized()).then(error => { + await this._processManager.createProcess(this._shellLaunchConfig, this._cols || Constants.DefaultCols, this._rows || Constants.DefaultRows, this._accessibilityService.isScreenReaderOptimized()).then(error => { if (error) { this._onProcessExit(error); } From 5e159d1b9005a9041109a10fad0504518c75db07 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 14 Jul 2021 14:44:35 -0700 Subject: [PATCH 2/2] Resize terminal when it's shown This fixes the following case: 1. Create and split the terminal 2. Hide the panel 3. Create a terminal in the background 4. Show it, the dimensions were wrong --- .../workbench/contrib/terminal/browser/terminalInstance.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index e22c8d14b4266..1824f9b66957d 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -1060,6 +1060,11 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { this._wrapperElement.classList.toggle('active', visible); } if (visible && this._xterm && this._xtermCore) { + // Resize to re-evaluate dimensions, this will ensure when switching to a terminal it is + // using the most up to date dimensions (eg. when terminal is created in the background + // using cached dimensions of a split terminal). + this._resize(); + // Trigger a manual scroll event which will sync the viewport and scroll bar. This is // necessary if the number of rows in the terminal has decreased while it was in the // background since scrollTop changes take no effect but the terminal's position does