From da68afbe53eebbadfd56037981b11a90eab3edc8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 7 Feb 2024 12:22:23 +0000 Subject: [PATCH 1/2] Use terminal data write event to figure out whether shell integration is working --- .../shellIntegrationService.ts | 36 ++++++++++++++++--- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/src/client/terminals/envCollectionActivation/shellIntegrationService.ts b/src/client/terminals/envCollectionActivation/shellIntegrationService.ts index bfa963537ec9..ffcdd1f1c350 100644 --- a/src/client/terminals/envCollectionActivation/shellIntegrationService.ts +++ b/src/client/terminals/envCollectionActivation/shellIntegrationService.ts @@ -2,10 +2,15 @@ // Licensed under the MIT License. import { injectable, inject } from 'inversify'; -import { IApplicationShell, ITerminalManager, IWorkspaceService } from '../../common/application/types'; +import { + IApplicationEnvironment, + IApplicationShell, + ITerminalManager, + IWorkspaceService, +} from '../../common/application/types'; import { identifyShellFromShellPath } from '../../common/terminal/shellDetectors/baseShellDetector'; import { TerminalShellType } from '../../common/terminal/types'; -import { IPersistentStateFactory } from '../../common/types'; +import { IDisposableRegistry, IPersistentStateFactory } from '../../common/types'; import { createDeferred, sleep } from '../../common/utils/async'; import { cache } from '../../common/utils/decorators'; import { traceError, traceInfo, traceVerbose } from '../../logging'; @@ -34,12 +39,29 @@ export class ShellIntegrationService implements IShellIntegrationService { */ private readonly USE_COMMAND_APPROACH = false; + private isWorkingForShell = new Set(); + constructor( @inject(ITerminalManager) private readonly terminalManager: ITerminalManager, @inject(IApplicationShell) private readonly appShell: IApplicationShell, @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, @inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory, - ) {} + @inject(IApplicationShell) private readonly appEnvironment: IApplicationEnvironment, + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, + ) { + this.disposables.push( + this.appShell.onDidWriteTerminalData((e) => { + if (e.data.includes('\x1b]633;A\x07')) { + let { shell } = this.appEnvironment; + if ('shellPath' in e.terminal.creationOptions && e.terminal.creationOptions.shellPath) { + shell = e.terminal.creationOptions.shellPath; + } + const shellType = identifyShellFromShellPath(shell); + this.isWorkingForShell.add(shellType); + } + }), + ); + } public async isWorking(shell: string): Promise { return this._isWorking(shell).catch((ex) => { @@ -62,8 +84,12 @@ export class ShellIntegrationService implements IShellIntegrationService { return false; } if (!this.USE_COMMAND_APPROACH) { - // For now, based on problems with using the command approach, assume it always works. - return true; + // For now, based on problems with using the command approach, use terminal data write event. + if (!this.isWorkingForShell.has(shellType)) { + // Maybe data write event has not been processed yet, wait a bit. + await sleep(1000); + } + return this.isWorkingForShell.has(shellType); } const key = `${isShellIntegrationWorkingKey}_${shellType}`; const persistedResult = this.persistentStateFactory.createGlobalPersistentState(key); From 8b105500b92c9d2efc0132cf207f75a1a6330519 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 7 Feb 2024 14:58:28 +0000 Subject: [PATCH 2/2] Don't do it for powershell --- .../shellIntegrationService.ts | 69 +++++++++++++++---- src/client/terminals/types.ts | 1 + 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/src/client/terminals/envCollectionActivation/shellIntegrationService.ts b/src/client/terminals/envCollectionActivation/shellIntegrationService.ts index ffcdd1f1c350..bb5168bc6e09 100644 --- a/src/client/terminals/envCollectionActivation/shellIntegrationService.ts +++ b/src/client/terminals/envCollectionActivation/shellIntegrationService.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import { injectable, inject } from 'inversify'; +import { EventEmitter } from 'vscode'; import { IApplicationEnvironment, IApplicationShell, @@ -41,28 +42,54 @@ export class ShellIntegrationService implements IShellIntegrationService { private isWorkingForShell = new Set(); + private readonly didChange = new EventEmitter(); + + private isDataWriteEventWorking = true; + constructor( @inject(ITerminalManager) private readonly terminalManager: ITerminalManager, @inject(IApplicationShell) private readonly appShell: IApplicationShell, @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, @inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory, - @inject(IApplicationShell) private readonly appEnvironment: IApplicationEnvironment, + @inject(IApplicationEnvironment) private readonly appEnvironment: IApplicationEnvironment, @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, ) { - this.disposables.push( - this.appShell.onDidWriteTerminalData((e) => { - if (e.data.includes('\x1b]633;A\x07')) { - let { shell } = this.appEnvironment; - if ('shellPath' in e.terminal.creationOptions && e.terminal.creationOptions.shellPath) { - shell = e.terminal.creationOptions.shellPath; + try { + this.appShell.onDidWriteTerminalData( + (e) => { + if (e.data.includes('\x1b]633;A\x07')) { + let { shell } = this.appEnvironment; + if ('shellPath' in e.terminal.creationOptions && e.terminal.creationOptions.shellPath) { + shell = e.terminal.creationOptions.shellPath; + } + const shellType = identifyShellFromShellPath(shell); + const wasWorking = this.isWorkingForShell.has(shellType); + this.isWorkingForShell.add(shellType); + if (!wasWorking) { + // If it wasn't working previously, status has changed. + this.didChange.fire(); + } } - const shellType = identifyShellFromShellPath(shell); - this.isWorkingForShell.add(shellType); - } - }), - ); + }, + this, + this.disposables, + ); + this.appEnvironment.onDidChangeShell( + async (shell: string) => { + this.createDummyHiddenTerminal(shell); + }, + this, + this.disposables, + ); + this.createDummyHiddenTerminal(this.appEnvironment.shell); + } catch (ex) { + this.isDataWriteEventWorking = false; + traceError('Unable to check if shell integration is active', ex); + } } + public readonly onDidChangeStatus = this.didChange.event; + public async isWorking(shell: string): Promise { return this._isWorking(shell).catch((ex) => { traceError(`Failed to determine if shell supports shell integration`, shell, ex); @@ -85,6 +112,14 @@ export class ShellIntegrationService implements IShellIntegrationService { } if (!this.USE_COMMAND_APPROACH) { // For now, based on problems with using the command approach, use terminal data write event. + if (!this.isDataWriteEventWorking) { + // Assume shell integration is working, if data write event isn't working. + return true; + } + if (shellType === TerminalShellType.powershell || shellType === TerminalShellType.powershellCore) { + // Due to upstream bug: https://github.com/microsoft/vscode/issues/204616, assume shell integration is working for now. + return true; + } if (!this.isWorkingForShell.has(shellType)) { // Maybe data write event has not been processed yet, wait a bit. await sleep(1000); @@ -102,6 +137,16 @@ export class ShellIntegrationService implements IShellIntegrationService { return result; } + /** + * Creates a dummy terminal so that we are guaranteed a data write event for this shell type. + */ + private createDummyHiddenTerminal(shell: string) { + this.terminalManager.createTerminal({ + shellPath: shell, + hideFromUser: true, + }); + } + private async checkIfWorkingByRunningCommand(shell: string): Promise { const shellType = identifyShellFromShellPath(shell); const deferred = createDeferred(); diff --git a/src/client/terminals/types.ts b/src/client/terminals/types.ts index 0fb268fe192c..fbdd490c175c 100644 --- a/src/client/terminals/types.ts +++ b/src/client/terminals/types.ts @@ -44,6 +44,7 @@ export interface ITerminalEnvVarCollectionService { export const IShellIntegrationService = Symbol('IShellIntegrationService'); export interface IShellIntegrationService { + onDidChangeStatus: Event; isWorking(shell: string): Promise; }