Skip to content

Commit 6979e72

Browse files
TyriarTyriar
authored
Merge pull request microsoft#285039 from microsoft/tyriar/285032__285031
Fix performance problems with terminal tabs
2 parents 64929b8 + 64c7e39 commit 6979e72

File tree

3 files changed

+81
-79
lines changed

3 files changed

+81
-79
lines changed

‎src/vs/workbench/contrib/terminal/browser/terminalInstance.ts‎

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ import type { IMenu } from '../../../../platform/actions/common/actions.js';
9090
import { IContextMenuService } from '../../../../platform/contextview/browser/contextView.js';
9191
import { TerminalContribCommandId } from '../terminalContribExports.js';
9292
import type { IProgressState } from '@xterm/addon-progress';
93-
import { refreshShellIntegrationInfoStatus } from './terminalTooltip.js';
9493
import { generateUuid } from '../../../../base/common/uuid.js';
9594
import { PromptInputState } from '../../../../platform/terminal/common/capabilities/commandDetection/promptInputModel.js';
9695
import { hasKey, isNumber, isString } from '../../../../base/common/types.js';
@@ -465,7 +464,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
465464
capabilityListeners.get(e.id)?.dispose();
466465
const refreshInfo = () => {
467466
this._labelComputer?.refreshLabel(this);
468-
refreshShellIntegrationInfoStatus(this);
467+
this._refreshShellIntegrationInfoStatus(this);
469468
};
470469
switch (e.id) {
471470
case TerminalCapability.CwdDetection: {
@@ -499,7 +498,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
499498
}
500499
}
501500
}));
502-
this._register(this.onDidChangeShellType(() => refreshShellIntegrationInfoStatus(this)));
501+
this._register(this.onDidChangeShellType(() => this._refreshShellIntegrationInfoStatus(this)));
503502
this._register(this.capabilities.onDidRemoveCapability(e => {
504503
capabilityListeners.get(e.id)?.dispose();
505504
}));
@@ -888,7 +887,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
888887
// Register and update the terminal's shell integration status
889888
this._register(Event.runAndSubscribe(xterm.shellIntegration.onDidChangeSeenSequences, () => {
890889
if (xterm.shellIntegration.seenSequences.size > 0) {
891-
refreshShellIntegrationInfoStatus(this);
890+
this._refreshShellIntegrationInfoStatus(this);
892891
}
893892
}));
894893

@@ -922,6 +921,54 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
922921
return xterm;
923922
}
924923

924+
// Debounce this to avoid impacting input latency while typing into the prompt
925+
@debounce(500)
926+
private _refreshShellIntegrationInfoStatus(instance: ITerminalInstance) {
927+
if (!instance.xterm) {
928+
return;
929+
}
930+
const cmdDetectionType = (
931+
instance.capabilities.get(TerminalCapability.CommandDetection)?.hasRichCommandDetection
932+
? nls.localize('shellIntegration.rich', 'Rich')
933+
: instance.capabilities.has(TerminalCapability.CommandDetection)
934+
? nls.localize('shellIntegration.basic', 'Basic')
935+
: instance.usedShellIntegrationInjection
936+
? nls.localize('shellIntegration.injectionFailed', "Injection failed to activate")
937+
: nls.localize('shellIntegration.no', 'No')
938+
);
939+
940+
const detailedAdditions: string[] = [];
941+
if (instance.shellType) {
942+
detailedAdditions.push(`Shell type: \`${instance.shellType}\``);
943+
}
944+
const cwd = instance.cwd;
945+
if (cwd) {
946+
detailedAdditions.push(`Current working directory: \`${cwd}\``);
947+
}
948+
const seenSequences = Array.from(instance.xterm.shellIntegration.seenSequences);
949+
if (seenSequences.length > 0) {
950+
detailedAdditions.push(`Seen sequences: ${seenSequences.map(e => `\`${e}\``).join(', ')}`);
951+
}
952+
const promptType = instance.capabilities.get(TerminalCapability.PromptTypeDetection)?.promptType;
953+
if (promptType) {
954+
detailedAdditions.push(`Prompt type: \`${promptType}\``);
955+
}
956+
const combinedString = instance.capabilities.get(TerminalCapability.CommandDetection)?.promptInputModel.getCombinedString();
957+
if (combinedString !== undefined) {
958+
detailedAdditions.push(`Prompt input: \`\`\`${combinedString}\`\`\``);
959+
}
960+
const detailedAdditionsString = detailedAdditions.length > 0
961+
? '\n\n' + detailedAdditions.map(e => `- ${e}`).join('\n')
962+
: '';
963+
964+
instance.statusList.add({
965+
id: TerminalStatus.ShellIntegrationInfo,
966+
severity: Severity.Info,
967+
tooltip: `${nls.localize('shellIntegration', "Shell integration")}: ${cmdDetectionType}`,
968+
detailedTooltip: `${nls.localize('shellIntegration', "Shell integration")}: ${cmdDetectionType}${detailedAdditionsString}`
969+
});
970+
}
971+
925972
async runCommand(commandLine: string, shouldExecute: boolean, commandId?: string): Promise<void> {
926973
let commandDetection = this.capabilities.get(TerminalCapability.CommandDetection);
927974
const siInjectionEnabled = this._configurationService.getValue(TerminalSettingId.ShellIntegrationEnabled) === true;

‎src/vs/workbench/contrib/terminal/browser/terminalTabsList.ts‎

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,12 @@ export class TerminalTabList extends WorkbenchList<ITerminalInstance> {
7474
private _terminalTabsSingleSelectedContextKey: IContextKey<boolean>;
7575
private _isSplitContextKey: IContextKey<boolean>;
7676

77+
private _hasText: boolean = true;
78+
get hasText(): boolean { return this._hasText; }
79+
80+
private _hasActionBar: boolean = true;
81+
get hasActionBar(): boolean { return this._hasActionBar; }
82+
7783
constructor(
7884
container: HTMLElement,
7985
@IContextKeyService contextKeyService: IContextKeyService,
@@ -94,7 +100,7 @@ export class TerminalTabList extends WorkbenchList<ITerminalInstance> {
94100
getHeight: () => TerminalTabsListSizes.TabHeight,
95101
getTemplateId: () => 'terminal.tabs'
96102
},
97-
[instantiationService.createInstance(TerminalTabsRenderer, container, instantiationService.createInstance(ResourceLabels, DEFAULT_LABELS_CONTAINER), () => this.getSelectedElements())],
103+
[instantiationService.createInstance(TerminalTabsRenderer, container, instantiationService.createInstance(ResourceLabels, DEFAULT_LABELS_CONTAINER), () => this.getSelectedElements(), () => this.hasText, () => this.hasActionBar)],
98104
{
99105
horizontalScrolling: false,
100106
supportDynamicHeights: false,
@@ -248,15 +254,29 @@ export class TerminalTabList extends WorkbenchList<ITerminalInstance> {
248254
const instance = this.getFocusedElements();
249255
this._isSplitContextKey.set(instance.length > 0 && this._terminalGroupService.instanceIsSplit(instance[0]));
250256
}
257+
258+
override layout(height?: number, width?: number): void {
259+
super.layout(height, width);
260+
const actualWidth = width ?? this.getHTMLElement().clientWidth;
261+
const newHasText = actualWidth >= TerminalTabsListSizes.MidpointViewWidth;
262+
const newHasActionBar = actualWidth > TerminalTabsListSizes.ActionbarMinimumWidth;
263+
if (this._hasText !== newHasText || this._hasActionBar !== newHasActionBar) {
264+
this._hasText = newHasText;
265+
this._hasActionBar = newHasActionBar;
266+
this.refresh();
267+
}
268+
}
251269
}
252270

253271
class TerminalTabsRenderer implements IListRenderer<ITerminalInstance, ITerminalTabEntryTemplate> {
254272
templateId = 'terminal.tabs';
255273

256274
constructor(
257-
private readonly _container: HTMLElement,
275+
_container: HTMLElement,
258276
private readonly _labels: ResourceLabels,
259277
private readonly _getSelection: () => ITerminalInstance[],
278+
private readonly _getHasText: () => boolean,
279+
private readonly _getHasActionBar: () => boolean,
260280
@IInstantiationService private readonly _instantiationService: IInstantiationService,
261281
@ITerminalConfigurationService private readonly _terminalConfigurationService: ITerminalConfigurationService,
262282
@ITerminalService private readonly _terminalService: ITerminalService,
@@ -321,25 +341,9 @@ class TerminalTabsRenderer implements IListRenderer<ITerminalInstance, ITerminal
321341
};
322342
}
323343

324-
shouldHideText(): boolean {
325-
return this._container ? this.getContainerWidthCachedForTask() < TerminalTabsListSizes.MidpointViewWidth : false;
326-
}
327-
328-
shouldHideActionBar(): boolean {
329-
return this._container ? this.getContainerWidthCachedForTask() <= TerminalTabsListSizes.ActionbarMinimumWidth : false;
330-
}
331-
332-
private _cachedContainerWidth = -1;
333-
getContainerWidthCachedForTask(): number {
334-
if (this._cachedContainerWidth === -1) {
335-
this._cachedContainerWidth = this._container.clientWidth;
336-
queueMicrotask(() => this._cachedContainerWidth = -1);
337-
}
338-
return this._cachedContainerWidth;
339-
}
340-
341344
renderElement(instance: ITerminalInstance, index: number, template: ITerminalTabEntryTemplate): void {
342-
const hasText = !this.shouldHideText();
345+
const hasText = this._getHasText();
346+
const hasActionBar = this._getHasActionBar();
343347

344348
const group = this._terminalGroupService.getGroupForInstance(instance);
345349
if (!group) {
@@ -365,7 +369,6 @@ class TerminalTabsRenderer implements IListRenderer<ITerminalInstance, ITerminal
365369
template.context.hoverActions = hoverInfo.actions;
366370

367371
const iconId = this._instantiationService.invokeFunction(getIconId, instance);
368-
const hasActionbar = !this.shouldHideActionBar();
369372
let label: string = '';
370373
if (!hasText) {
371374
const primaryStatus = instance.statusList.primary;
@@ -385,7 +388,7 @@ class TerminalTabsRenderer implements IListRenderer<ITerminalInstance, ITerminal
385388
}
386389
}
387390

388-
if (!hasActionbar) {
391+
if (!hasActionBar) {
389392
template.actionBar.clear();
390393
}
391394

‎src/vs/workbench/contrib/terminal/browser/terminalTooltip.ts‎

Lines changed: 5 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,15 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { localize } from '../../../../nls.js';
7-
import { ITerminalInstance } from './terminal.js';
6+
import type { IHoverAction } from '../../../../base/browser/ui/hover/hover.js';
87
import { asArray } from '../../../../base/common/arrays.js';
98
import { MarkdownString } from '../../../../base/common/htmlContent.js';
10-
import type { IHoverAction } from '../../../../base/browser/ui/hover/hover.js';
11-
import { TerminalCapability } from '../../../../platform/terminal/common/capabilities/capabilities.js';
12-
import { TerminalStatus } from './terminalStatusList.js';
13-
import Severity from '../../../../base/common/severity.js';
9+
import { basename } from '../../../../base/common/path.js';
10+
import { localize } from '../../../../nls.js';
1411
import { StorageScope, StorageTarget, type IStorageService } from '../../../../platform/storage/common/storage.js';
15-
import { TerminalStorageKeys } from '../common/terminalStorageKeys.js';
1612
import type { ITerminalStatusHoverAction } from '../common/terminal.js';
17-
import { basename } from '../../../../base/common/path.js';
13+
import { TerminalStorageKeys } from '../common/terminalStorageKeys.js';
14+
import { ITerminalInstance } from './terminal.js';
1815

1916
export function getInstanceHoverInfo(instance: ITerminalInstance, storageService: IStorageService): { content: MarkdownString; actions: IHoverAction[] } {
2017
const showDetailed = parseInt(storageService.get(TerminalStorageKeys.TabsShowDetailed, StorageScope.APPLICATION) ?? '0');
@@ -77,48 +74,3 @@ export function getShellProcessTooltip(instance: ITerminalInstance, showDetailed
7774
return lines.length ? `\n\n---\n\n${lines.join('\n')}` : '';
7875
}
7976

80-
export function refreshShellIntegrationInfoStatus(instance: ITerminalInstance) {
81-
if (!instance.xterm) {
82-
return;
83-
}
84-
const cmdDetectionType = (
85-
instance.capabilities.get(TerminalCapability.CommandDetection)?.hasRichCommandDetection
86-
? localize('shellIntegration.rich', 'Rich')
87-
: instance.capabilities.has(TerminalCapability.CommandDetection)
88-
? localize('shellIntegration.basic', 'Basic')
89-
: instance.usedShellIntegrationInjection
90-
? localize('shellIntegration.injectionFailed', "Injection failed to activate")
91-
: localize('shellIntegration.no', 'No')
92-
);
93-
94-
const detailedAdditions: string[] = [];
95-
if (instance.shellType) {
96-
detailedAdditions.push(`Shell type: \`${instance.shellType}\``);
97-
}
98-
const cwd = instance.cwd;
99-
if (cwd) {
100-
detailedAdditions.push(`Current working directory: \`${cwd}\``);
101-
}
102-
const seenSequences = Array.from(instance.xterm.shellIntegration.seenSequences);
103-
if (seenSequences.length > 0) {
104-
detailedAdditions.push(`Seen sequences: ${seenSequences.map(e => `\`${e}\``).join(', ')}`);
105-
}
106-
const promptType = instance.capabilities.get(TerminalCapability.PromptTypeDetection)?.promptType;
107-
if (promptType) {
108-
detailedAdditions.push(`Prompt type: \`${promptType}\``);
109-
}
110-
const combinedString = instance.capabilities.get(TerminalCapability.CommandDetection)?.promptInputModel.getCombinedString();
111-
if (combinedString !== undefined) {
112-
detailedAdditions.push(`Prompt input: \`\`\`${combinedString}\`\`\``);
113-
}
114-
const detailedAdditionsString = detailedAdditions.length > 0
115-
? '\n\n' + detailedAdditions.map(e => `- ${e}`).join('\n')
116-
: '';
117-
118-
instance.statusList.add({
119-
id: TerminalStatus.ShellIntegrationInfo,
120-
severity: Severity.Info,
121-
tooltip: `${localize('shellIntegration', "Shell integration")}: ${cmdDetectionType}`,
122-
detailedTooltip: `${localize('shellIntegration', "Shell integration")}: ${cmdDetectionType}${detailedAdditionsString}`
123-
});
124-
}

0 commit comments

Comments
 (0)