Skip to content

Commit

Permalink
Enable shell integration in hidden terminals
Browse files Browse the repository at this point in the history
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 microsoft#199611
Part of microsoft#197187
  • Loading branch information
Tyriar committed Mar 18, 2024
1 parent 7015a87 commit f5f392c
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/vs/platform/terminal/common/terminalEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
15 changes: 13 additions & 2 deletions src/vs/platform/terminal/node/terminalEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit f5f392c

Please sign in to comment.