Skip to content

Commit 2fc9fea

Browse files
author
Kartik Raj
authored
Revert "Reliably detect whether shell integration is working" (#22445)
Reverts #22440 It seems reactivating never finishes after this, although this doesn't repro when debugging the extension, have to investigate further.
1 parent f98caf6 commit 2fc9fea

File tree

11 files changed

+44
-183
lines changed

11 files changed

+44
-183
lines changed

package.json

+1-2
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@
2323
"testObserver",
2424
"quickPickItemTooltip",
2525
"saveEditor",
26-
"terminalDataWriteEvent",
27-
"terminalExecuteCommandEvent"
26+
"terminalDataWriteEvent"
2827
],
2928
"author": {
3029
"name": "Microsoft Corporation"

src/client/common/application/applicationShell.ts

+1-9
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import {
3939
WorkspaceFolderPickOptions,
4040
} from 'vscode';
4141
import { traceError } from '../../logging';
42-
import { IApplicationShell, TerminalDataWriteEvent, TerminalExecutedCommand } from './types';
42+
import { IApplicationShell, TerminalDataWriteEvent } from './types';
4343

4444
@injectable()
4545
export class ApplicationShell implements IApplicationShell {
@@ -182,12 +182,4 @@ export class ApplicationShell implements IApplicationShell {
182182
return new EventEmitter<TerminalDataWriteEvent>().event;
183183
}
184184
}
185-
public get onDidExecuteTerminalCommand(): Event<TerminalExecutedCommand> | undefined {
186-
try {
187-
return window.onDidExecuteTerminalCommand;
188-
} catch (ex) {
189-
traceError('Failed to get proposed API TerminalExecutedCommand', ex);
190-
return undefined;
191-
}
192-
}
193185
}

src/client/common/application/types.ts

-34
Original file line numberDiff line numberDiff line change
@@ -78,42 +78,8 @@ export interface TerminalDataWriteEvent {
7878
readonly data: string;
7979
}
8080

81-
export interface TerminalExecutedCommand {
82-
/**
83-
* The {@link Terminal} the command was executed in.
84-
*/
85-
terminal: Terminal;
86-
/**
87-
* The full command line that was executed, including both the command and the arguments.
88-
*/
89-
commandLine: string | undefined;
90-
/**
91-
* The current working directory that was reported by the shell. This will be a {@link Uri}
92-
* if the string reported by the shell can reliably be mapped to the connected machine.
93-
*/
94-
cwd: Uri | string | undefined;
95-
/**
96-
* The exit code reported by the shell.
97-
*/
98-
exitCode: number | undefined;
99-
/**
100-
* The output of the command when it has finished executing. This is the plain text shown in
101-
* the terminal buffer and does not include raw escape sequences. Depending on the shell
102-
* setup, this may include the command line as part of the output.
103-
*/
104-
output: string | undefined;
105-
}
106-
10781
export const IApplicationShell = Symbol('IApplicationShell');
10882
export interface IApplicationShell {
109-
/**
110-
* An event that is emitted when a terminal with shell integration activated has completed
111-
* executing a command.
112-
*
113-
* Note that this event will not fire if the executed command exits the shell, listen to
114-
* {@link onDidCloseTerminal} to handle that case.
115-
*/
116-
readonly onDidExecuteTerminalCommand: Event<TerminalExecutedCommand> | undefined;
11783
/**
11884
* An [event](#Event) which fires when the focus state of the current window
11985
* changes. The value of the event represents whether the window is focused.

src/client/terminals/envCollectionActivation/service.ts

+22-7
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ import { TerminalShellType } from '../../common/terminal/types';
3737
import { OSType } from '../../common/utils/platform';
3838
import { normCase } from '../../common/platform/fs-paths';
3939
import { PythonEnvType } from '../../pythonEnvironments/base/info';
40-
import { IShellIntegrationService, ITerminalEnvVarCollectionService } from '../types';
40+
import { ITerminalEnvVarCollectionService } from '../types';
41+
import { ShellIntegrationShells } from './shellIntegration';
4142
import { ProgressService } from '../../common/application/progressService';
4243

4344
@injectable()
@@ -79,7 +80,6 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
7980
@inject(IWorkspaceService) private workspaceService: IWorkspaceService,
8081
@inject(IConfigurationService) private readonly configurationService: IConfigurationService,
8182
@inject(IPathUtils) private readonly pathUtils: IPathUtils,
82-
@inject(IShellIntegrationService) private readonly shellIntegrationService: IShellIntegrationService,
8383
) {
8484
this.separator = platform.osType === OSType.Windows ? ';' : ':';
8585
this.progressService = new ProgressService(this.shell);
@@ -121,7 +121,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
121121
this.disposables,
122122
);
123123
const { shell } = this.applicationEnvironment;
124-
const isActive = this.shellIntegrationService.isWorking(shell);
124+
const isActive = this.isShellIntegrationActive(shell);
125125
const shellType = identifyShellFromShellPath(shell);
126126
if (!isActive && shellType !== TerminalShellType.commandPrompt) {
127127
traceWarn(`Shell integration is not active, environment activated maybe overriden by the shell.`);
@@ -184,7 +184,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
184184

185185
// PS1 in some cases is a shell variable (not an env variable) so "env" might not contain it, calculate it in that case.
186186
env.PS1 = await this.getPS1(shell, resource, env);
187-
const prependOptions = await this.getPrependOptions(shell);
187+
const prependOptions = this.getPrependOptions(shell);
188188

189189
// Clear any previously set env vars from collection
190190
envVarCollection.clear();
@@ -277,7 +277,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
277277
// PS1 should be set but no PS1 was set.
278278
return;
279279
}
280-
const config = await this.shellIntegrationService.isWorking(shell);
280+
const config = this.isShellIntegrationActive(shell);
281281
if (!config) {
282282
traceVerbose('PS1 is not set when shell integration is disabled.');
283283
return;
@@ -332,8 +332,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
332332
}
333333
}
334334

335-
private async getPrependOptions(shell: string): Promise<EnvironmentVariableMutatorOptions> {
336-
const isActive = await this.shellIntegrationService.isWorking(shell);
335+
private getPrependOptions(shell: string): EnvironmentVariableMutatorOptions {
336+
const isActive = this.isShellIntegrationActive(shell);
337337
// Ideally we would want to prepend exactly once, either at shell integration or process creation.
338338
// TODO: Stop prepending altogether once https://github.com/microsoft/vscode/issues/145234 is available.
339339
return isActive
@@ -347,6 +347,21 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
347347
};
348348
}
349349

350+
private isShellIntegrationActive(shell: string): boolean {
351+
const isEnabled = this.workspaceService
352+
.getConfiguration('terminal')
353+
.get<boolean>('integrated.shellIntegration.enabled')!;
354+
if (isEnabled && ShellIntegrationShells.includes(identifyShellFromShellPath(shell))) {
355+
// Unfortunately shell integration could still've failed in remote scenarios, we can't know for sure:
356+
// https://code.visualstudio.com/docs/terminal/shell-integration#_automatic-script-injection
357+
return true;
358+
}
359+
if (!isEnabled) {
360+
traceVerbose('Shell integrated is disabled in user settings.');
361+
}
362+
return false;
363+
}
364+
350365
private getEnvironmentVariableCollection(scope: EnvironmentVariableScope = {}) {
351366
const envVarCollection = this.context.environmentVariableCollection as GlobalEnvironmentVariableCollection;
352367
return envVarCollection.getScoped(scope);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { TerminalShellType } from '../../common/terminal/types';
2+
3+
/**
4+
* This is a list of shells which support shell integration:
5+
* https://code.visualstudio.com/docs/terminal/shell-integration
6+
*/
7+
export const ShellIntegrationShells = [
8+
TerminalShellType.powershell,
9+
TerminalShellType.powershellCore,
10+
TerminalShellType.bash,
11+
TerminalShellType.zsh,
12+
TerminalShellType.fish,
13+
];

src/client/terminals/envCollectionActivation/shellIntegrationService.ts

-68
This file was deleted.

src/client/terminals/serviceRegistry.ts

-3
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,13 @@ import {
1212
ICodeExecutionHelper,
1313
ICodeExecutionManager,
1414
ICodeExecutionService,
15-
IShellIntegrationService,
1615
ITerminalAutoActivation,
1716
ITerminalEnvVarCollectionService,
1817
} from './types';
1918
import { TerminalEnvVarCollectionService } from './envCollectionActivation/service';
2019
import { IExtensionActivationService, IExtensionSingleActivationService } from '../activation/types';
2120
import { TerminalDeactivateLimitationPrompt } from './envCollectionActivation/deactivatePrompt';
2221
import { TerminalIndicatorPrompt } from './envCollectionActivation/indicatorPrompt';
23-
import { ShellIntegrationService } from './envCollectionActivation/shellIntegrationService';
2422

2523
export function registerTypes(serviceManager: IServiceManager): void {
2624
serviceManager.addSingleton<ICodeExecutionHelper>(ICodeExecutionHelper, CodeExecutionHelper);
@@ -52,6 +50,5 @@ export function registerTypes(serviceManager: IServiceManager): void {
5250
IExtensionSingleActivationService,
5351
TerminalDeactivateLimitationPrompt,
5452
);
55-
serviceManager.addSingleton<IShellIntegrationService>(IShellIntegrationService, ShellIntegrationService);
5653
serviceManager.addBinding(ITerminalEnvVarCollectionService, IExtensionActivationService);
5754
}

src/client/terminals/types.ts

-5
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,3 @@ export interface ITerminalEnvVarCollectionService {
4141
*/
4242
isTerminalPromptSetCorrectly(resource?: Resource): boolean;
4343
}
44-
45-
export const IShellIntegrationService = Symbol('IShellIntegrationService');
46-
export interface IShellIntegrationService {
47-
isWorking(shell: string): Promise<boolean>;
48-
}

src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts

+7-7
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
GlobalEnvironmentVariableCollection,
1313
ProgressLocation,
1414
Uri,
15+
WorkspaceConfiguration,
1516
WorkspaceFolder,
1617
} from 'vscode';
1718
import {
@@ -37,7 +38,6 @@ import { IInterpreterService } from '../../../client/interpreter/contracts';
3738
import { PathUtils } from '../../../client/common/platform/pathUtils';
3839
import { PythonEnvType } from '../../../client/pythonEnvironments/base/info';
3940
import { PythonEnvironment } from '../../../client/pythonEnvironments/info';
40-
import { IShellIntegrationService } from '../../../client/terminals/types';
4141

4242
suite('Terminal Environment Variable Collection Service', () => {
4343
let platform: IPlatformService;
@@ -50,28 +50,29 @@ suite('Terminal Environment Variable Collection Service', () => {
5050
let applicationEnvironment: IApplicationEnvironment;
5151
let environmentActivationService: IEnvironmentActivationService;
5252
let workspaceService: IWorkspaceService;
53+
let workspaceConfig: WorkspaceConfiguration;
5354
let terminalEnvVarCollectionService: TerminalEnvVarCollectionService;
5455
const progressOptions = {
5556
location: ProgressLocation.Window,
5657
title: Interpreters.activatingTerminals,
5758
};
5859
let configService: IConfigurationService;
59-
let shellIntegrationService: IShellIntegrationService;
6060
const displayPath = 'display/path';
6161
const customShell = 'powershell';
6262
const defaultShell = defaultShells[getOSType()];
6363

6464
setup(() => {
6565
workspaceService = mock<IWorkspaceService>();
66+
workspaceConfig = mock<WorkspaceConfiguration>();
6667
when(workspaceService.getWorkspaceFolder(anything())).thenReturn(undefined);
6768
when(workspaceService.workspaceFolders).thenReturn(undefined);
69+
when(workspaceService.getConfiguration('terminal')).thenReturn(instance(workspaceConfig));
70+
when(workspaceConfig.get<boolean>('integrated.shellIntegration.enabled')).thenReturn(true);
6871
platform = mock<IPlatformService>();
6972
when(platform.osType).thenReturn(getOSType());
7073
interpreterService = mock<IInterpreterService>();
7174
context = mock<IExtensionContext>();
7275
shell = mock<IApplicationShell>();
73-
shellIntegrationService = mock<IShellIntegrationService>();
74-
when(shellIntegrationService.isWorking(anything())).thenResolve(true);
7576
globalCollection = mock<GlobalEnvironmentVariableCollection>();
7677
collection = mock<EnvironmentVariableCollection>();
7778
when(context.environmentVariableCollection).thenReturn(instance(globalCollection));
@@ -107,7 +108,6 @@ suite('Terminal Environment Variable Collection Service', () => {
107108
instance(workspaceService),
108109
instance(configService),
109110
new PathUtils(getOSType() === OSType.Windows),
110-
instance(shellIntegrationService),
111111
);
112112
});
113113

@@ -445,8 +445,8 @@ suite('Terminal Environment Variable Collection Service', () => {
445445
});
446446

447447
test('Correct track that prompt was set for PS1 if shell integration is disabled', async () => {
448-
reset(shellIntegrationService);
449-
when(shellIntegrationService.isWorking(anything())).thenResolve(false);
448+
reset(workspaceConfig);
449+
when(workspaceConfig.get<boolean>('integrated.shellIntegration.enabled')).thenReturn(false);
450450
when(platform.osType).thenReturn(OSType.Linux);
451451
const envVars: NodeJS.ProcessEnv = { VIRTUAL_ENV: 'prefix/to/venv', PS1: '(.venv)', ...process.env };
452452
const ps1Shell = 'bash';

src/test/terminals/serviceRegistry.unit.test.ts

-3
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,11 @@ import { TerminalCodeExecutionProvider } from '../../client/terminals/codeExecut
1313
import { TerminalDeactivateLimitationPrompt } from '../../client/terminals/envCollectionActivation/deactivatePrompt';
1414
import { TerminalIndicatorPrompt } from '../../client/terminals/envCollectionActivation/indicatorPrompt';
1515
import { TerminalEnvVarCollectionService } from '../../client/terminals/envCollectionActivation/service';
16-
import { ShellIntegrationService } from '../../client/terminals/envCollectionActivation/shellIntegrationService';
1716
import { registerTypes } from '../../client/terminals/serviceRegistry';
1817
import {
1918
ICodeExecutionHelper,
2019
ICodeExecutionManager,
2120
ICodeExecutionService,
22-
IShellIntegrationService,
2321
ITerminalAutoActivation,
2422
ITerminalEnvVarCollectionService,
2523
} from '../../client/terminals/types';
@@ -37,7 +35,6 @@ suite('Terminal - Service Registry', () => {
3735
[ITerminalEnvVarCollectionService, TerminalEnvVarCollectionService],
3836
[IExtensionSingleActivationService, TerminalIndicatorPrompt],
3937
[IExtensionSingleActivationService, TerminalDeactivateLimitationPrompt],
40-
[IShellIntegrationService, ShellIntegrationService],
4138
].forEach((args) => {
4239
if (args.length === 2) {
4340
services

0 commit comments

Comments
 (0)