Skip to content

Do not show inherit env prompt for conda envs when running "remotely" #18922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/2 Fixes/18510.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Do not show inherit env prompt for conda envs when running "remotely".
3 changes: 3 additions & 0 deletions src/client/common/application/applicationEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ export class ApplicationEnvironment implements IApplicationEnvironment {
public get machineId(): string {
return vscode.env.machineId;
}
public get remoteName(): string | undefined {
return vscode.env.remoteName;
}
public get extensionName(): string {
return this.packageJson.displayName;
}
Expand Down
10 changes: 10 additions & 0 deletions src/client/common/application/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1072,6 +1072,16 @@ export interface IApplicationEnvironment {
* from a desktop application or a web browser.
*/
readonly uiKind: UIKind;
/**
* The name of a remote. Defined by extensions, popular samples are `wsl` for the Windows
* Subsystem for Linux or `ssh-remote` for remotes using a secure shell.
*
* *Note* that the value is `undefined` when there is no remote extension host but that the
* value is defined in all extension hosts (local and remote) in case a remote extension host
* exists. Use {@link Extension.extensionKind} to know if
* a specific extension runs remote or not.
*/
readonly remoteName: string | undefined;
}

export const ILanguageService = Symbol('ILanguageService');
Expand Down
8 changes: 7 additions & 1 deletion src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { inject, injectable, optional } from 'inversify';
import { ConfigurationTarget, Uri } from 'vscode';
import { IExtensionActivationService } from '../../activation/types';
import { IApplicationShell, IWorkspaceService } from '../../common/application/types';
import { IApplicationEnvironment, IApplicationShell, IWorkspaceService } from '../../common/application/types';
import { IPlatformService } from '../../common/platform/types';
import { IBrowserService, IPersistentStateFactory } from '../../common/types';
import { Common, Interpreters } from '../../common/utils/localize';
Expand All @@ -26,6 +26,7 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService {
@inject(IApplicationShell) private readonly appShell: IApplicationShell,
@inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory,
@inject(IPlatformService) private readonly platformService: IPlatformService,
@inject(IApplicationEnvironment) private readonly appEnvironment: IApplicationEnvironment,
@optional() public hasPromptBeenShownInCurrentSession: boolean = false,
) {}

Expand Down Expand Up @@ -76,6 +77,11 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService {
if (this.hasPromptBeenShownInCurrentSession) {
return false;
}
if (this.appEnvironment.remoteName) {
// `terminal.integrated.inheritEnv` is only applicable user scope, so won't apply
// in remote scenarios: https://github.com/microsoft/vscode/issues/147421
return false;
}
if (this.platformService.isWindows) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ import * as sinon from 'sinon';
import { instance, mock, verify, when } from 'ts-mockito';
import * as TypeMoq from 'typemoq';
import { ConfigurationTarget, Uri, WorkspaceConfiguration } from 'vscode';
import { IApplicationShell, IWorkspaceService } from '../../../client/common/application/types';
import {
IApplicationEnvironment,
IApplicationShell,
IWorkspaceService,
} from '../../../client/common/application/types';
import { PersistentStateFactory } from '../../../client/common/persistentState';
import { IPlatformService } from '../../../client/common/platform/types';
import { IBrowserService, IPersistentState, IPersistentStateFactory } from '../../../client/common/types';
Expand All @@ -28,6 +32,7 @@ suite('Conda Inherit Env Prompt', async () => {
let interpreterService: TypeMoq.IMock<IInterpreterService>;
let platformService: TypeMoq.IMock<IPlatformService>;
let browserService: TypeMoq.IMock<IBrowserService>;
let applicationEnvironment: TypeMoq.IMock<IApplicationEnvironment>;
let persistentStateFactory: IPersistentStateFactory;
let notificationPromptEnabled: TypeMoq.IMock<IPersistentState<any>>;
let condaInheritEnvPrompt: CondaInheritEnvPrompt;
Expand All @@ -45,13 +50,17 @@ suite('Conda Inherit Env Prompt', async () => {
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
persistentStateFactory = mock(PersistentStateFactory);
platformService = TypeMoq.Mock.ofType<IPlatformService>();
applicationEnvironment = TypeMoq.Mock.ofType<IApplicationEnvironment>();
applicationEnvironment.setup((a) => a.remoteName).returns(() => undefined);
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),

platformService.object,
applicationEnvironment.object,
);
});
test('Returns false if prompt has already been shown in the current session', async () => {
Expand All @@ -61,7 +70,9 @@ suite('Conda Inherit Env Prompt', async () => {
browserService.object,
appShell.object,
instance(persistentStateFactory),

platformService.object,
applicationEnvironment.object,
true,
);
const workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
Expand All @@ -78,6 +89,14 @@ suite('Conda Inherit Env Prompt', async () => {
expect(condaInheritEnvPrompt.hasPromptBeenShownInCurrentSession).to.equal(true, 'Should be true');
verifyAll();
});
test('Returns false if running on remote', async () => {
applicationEnvironment.reset();
applicationEnvironment.setup((a) => a.remoteName).returns(() => 'ssh');
const result = await condaInheritEnvPrompt.shouldShowPrompt(resource);
expect(result).to.equal(false, 'Prompt should not be shown');
expect(condaInheritEnvPrompt.hasPromptBeenShownInCurrentSession).to.equal(false, 'Should be false');
verifyAll();
});
test('Returns false if on Windows', async () => {
platformService
.setup((ps) => ps.isWindows)
Expand Down Expand Up @@ -245,6 +264,8 @@ suite('Conda Inherit Env Prompt', async () => {
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
persistentStateFactory = mock(PersistentStateFactory);
platformService = TypeMoq.Mock.ofType<IPlatformService>();
applicationEnvironment = TypeMoq.Mock.ofType<IApplicationEnvironment>();
applicationEnvironment.setup((a) => a.remoteName).returns(() => undefined);
});

teardown(() => {
Expand All @@ -261,7 +282,9 @@ suite('Conda Inherit Env Prompt', async () => {
browserService.object,
appShell.object,
instance(persistentStateFactory),

platformService.object,
applicationEnvironment.object,
);

const promise = condaInheritEnvPrompt.activate(resource);
Expand All @@ -285,7 +308,9 @@ suite('Conda Inherit Env Prompt', async () => {
browserService.object,
appShell.object,
instance(persistentStateFactory),

platformService.object,
applicationEnvironment.object,
);
await condaInheritEnvPrompt.activate(resource);
assert.ok(initializeInBackground.calledOnce);
Expand All @@ -302,6 +327,8 @@ suite('Conda Inherit Env Prompt', async () => {
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
persistentStateFactory = mock(PersistentStateFactory);
platformService = TypeMoq.Mock.ofType<IPlatformService>();
applicationEnvironment = TypeMoq.Mock.ofType<IApplicationEnvironment>();
applicationEnvironment.setup((a) => a.remoteName).returns(() => undefined);
});

teardown(() => {
Expand All @@ -319,7 +346,9 @@ suite('Conda Inherit Env Prompt', async () => {
browserService.object,
appShell.object,
instance(persistentStateFactory),

platformService.object,
applicationEnvironment.object,
);
await condaInheritEnvPrompt.initializeInBackground(resource);
assert.ok(shouldShowPrompt.calledOnce);
Expand All @@ -337,7 +366,9 @@ suite('Conda Inherit Env Prompt', async () => {
browserService.object,
appShell.object,
instance(persistentStateFactory),

platformService.object,
applicationEnvironment.object,
);
await condaInheritEnvPrompt.initializeInBackground(resource);
assert.ok(shouldShowPrompt.calledOnce);
Expand All @@ -355,6 +386,8 @@ suite('Conda Inherit Env Prompt', async () => {
browserService = TypeMoq.Mock.ofType<IBrowserService>();
notificationPromptEnabled = TypeMoq.Mock.ofType<IPersistentState<any>>();
platformService = TypeMoq.Mock.ofType<IPlatformService>();
applicationEnvironment = TypeMoq.Mock.ofType<IApplicationEnvironment>();
applicationEnvironment.setup((a) => a.remoteName).returns(() => undefined);
when(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).thenReturn(
notificationPromptEnabled.object,
);
Expand All @@ -364,7 +397,9 @@ suite('Conda Inherit Env Prompt', async () => {
browserService.object,
appShell.object,
instance(persistentStateFactory),

platformService.object,
applicationEnvironment.object,
);
});

Expand Down