From f5f392c2c44a06a1a966967c206e853a31467305 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 18 Mar 2024 11:58:10 -0700 Subject: [PATCH] Enable shell integration in hidden terminals As I understand it, hidden terminals were always exempty from shell integration because hidden terminals were thought of as private to an extension in a way, and also since they probably weren't going to be interacted with. Especially since the introduction of the environment variable collection API I think we should change this decision, not doing so will lead to weirdness for extensions where hidden ones (which may be shown later) aren't fully featured and get inconsistent envs. Fixes #199611 Part of #197187 --- .../terminal/common/terminalEnvironment.ts | 2 +- .../platform/terminal/node/terminalEnvironment.ts | 15 +++++++++++++-- .../contrib/terminal/browser/terminalInstance.ts | 2 +- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/vs/platform/terminal/common/terminalEnvironment.ts b/src/vs/platform/terminal/common/terminalEnvironment.ts index d2925819862fb..5ddf2aa71d641 100644 --- a/src/vs/platform/terminal/common/terminalEnvironment.ts +++ b/src/vs/platform/terminal/common/terminalEnvironment.ts @@ -66,5 +66,5 @@ export function sanitizeCwd(cwd: string): string { * @param slc The shell launch config to check. */ export function shouldUseEnvironmentVariableCollection(slc: IShellLaunchConfig): boolean { - return !slc.strictEnv && !slc.hideFromUser; + return !slc.strictEnv; } diff --git a/src/vs/platform/terminal/node/terminalEnvironment.ts b/src/vs/platform/terminal/node/terminalEnvironment.ts index 703e0fec62322..c59f1ddefd979 100644 --- a/src/vs/platform/terminal/node/terminalEnvironment.ts +++ b/src/vs/platform/terminal/node/terminalEnvironment.ts @@ -112,12 +112,23 @@ export function getShellIntegrationInjection( logService: ILogService, productService: IProductService ): IShellIntegrationConfigInjection | undefined { - // Shell integration arg injection is disabled when: + // Conditionally disable shell integration arg injection // - The global setting is disabled // - There is no executable (not sure what script to run) // - The terminal is used by a feature like tasks or debugging const useWinpty = isWindows && (!options.windowsEnableConpty || getWindowsBuildNumber() < 18309); - if (!options.shellIntegration.enabled || !shellLaunchConfig.executable || (shellLaunchConfig.isFeatureTerminal && !shellLaunchConfig.forceShellIntegration) || shellLaunchConfig.hideFromUser || shellLaunchConfig.ignoreShellIntegration || useWinpty) { + if ( + // The global setting is disabled + !options.shellIntegration.enabled || + // There is no executable (so there's no way to determine how to inject) + !shellLaunchConfig.executable || + // It's a feature terminal (tasks, debug), unless it's explicitly being forced + (shellLaunchConfig.isFeatureTerminal && !shellLaunchConfig.forceShellIntegration) || + // The ignoreShellIntegration flag is passed (eg. relaunching without shell integration) + shellLaunchConfig.ignoreShellIntegration || + // Winpty is unsupported + useWinpty + ) { return undefined; } diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index b24312642f401..9941d46b3a3ef 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -721,7 +721,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { throw new ErrorNoTelemetry('Terminal disposed of during xterm.js creation'); } - const disableShellIntegrationReporting = (this.shellLaunchConfig.hideFromUser || this.shellLaunchConfig.executable === undefined || this.shellType === undefined) || !shellIntegrationSupportedShellTypes.includes(this.shellType); + const disableShellIntegrationReporting = (this.shellLaunchConfig.executable === undefined || this.shellType === undefined) || !shellIntegrationSupportedShellTypes.includes(this.shellType); const xterm = this._scopedInstantiationService.createInstance( XtermTerminal, Terminal,