Improve select folder quickpick UI in multiroot workspaces (#18256)

Kartik Raj · web-flow · commit e9870c0c15da · 2022-01-07T23:02:10.000+05:30
* Add interpreter path text in select folder quickpick

* Add an option to clear interpreters for all folders

* Fix some tests

* News

* Localize

* Fix tests

* Add test
diff --git a/news/1 Enhancements/17693.md b/news/1 Enhancements/17693.md
@@ -0,0 +1 @@
+Add an option to clear interpreter setting for all workspace folders in multiroot scenario.
diff --git a/package.nls.json b/package.nls.json
@@ -46,6 +46,7 @@
     "Pylance.pylanceRevertToJedi": "Revert to Jedi",
     "Experiments.inGroup": "Experiment '{0}' is active",
     "Experiments.optedOutOf": "Experiment '{0}' is inactive",
+    "Interpreters.clearAtWorkspace" : "Clear at workspace level",
     "Interpreters.RefreshingInterpreters": "Refreshing Python Interpreters",
     "Interpreters.entireWorkspace": "Select at workspace level",
     "Interpreters.pythonInterpreterPath": "Python interpreter path: {0}",
@@ -61,6 +62,7 @@
     "InterpreterQuickPickList.browsePath.title": "Select Python interpreter",
     "InterpreterQuickPickList.defaultInterpreterPath.label": "Use Python from `python.defaultInterpreterPath` setting",
     "diagnostics.upgradeCodeRunner": "Please update the Code Runner extension for it to be compatible with the Python extension.",
+    "Common.clearAll" : "Clear all",
     "Common.bannerLabelYes": "Yes",
     "Common.bannerLabelNo": "No",
     "Common.doNotShowAgain": "Do not show again",
diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts
@@ -91,6 +91,7 @@ export namespace Common {
     export const and = localize('Common.and', 'and');
     export const reportThisIssue = localize('Common.reportThisIssue', 'Report this issue');
     export const recommended = localize('Common.recommended', 'Recommended');
+    export const clearAll = localize('Common.clearAll', 'Clear all');
 }
 
 export namespace CommonSurvey {
@@ -282,6 +283,7 @@ export namespace Interpreters {
         'We noticed a new virtual environment has been created. Do you want to select it for the workspace folder?',
     );
     export const entireWorkspace = localize('Interpreters.entireWorkspace', 'Select at workspace level');
+    export const clearAtWorkspace = localize('Interpreters.clearAtWorkspace', 'Clear at workspace level');
     export const selectInterpreterTip = localize(
         'Interpreters.selectInterpreterTip',
         'Tip: you can change the Python interpreter used by the Python extension by clicking on the Python version in the status bar',
diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/base.ts b/src/client/interpreter/configuration/interpreterSelector/commands/base.ts
@@ -8,10 +8,12 @@ import * as path from 'path';
 import { ConfigurationTarget, Disposable, QuickPickItem, Uri } from 'vscode';
 import { IExtensionSingleActivationService } from '../../../../activation/types';
 import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../common/application/types';
-import { IDisposable, Resource } from '../../../../common/types';
-import { Interpreters } from '../../../../common/utils/localize';
+import { IConfigurationService, IDisposable, IPathUtils, Resource } from '../../../../common/types';
+import { Common, Interpreters } from '../../../../common/utils/localize';
 import { IPythonPathUpdaterServiceManager } from '../../types';
-
+export interface WorkspaceSelectionQuickPickItem extends QuickPickItem {
+    uri?: Uri;
+}
 @injectable()
 export abstract class BaseInterpreterSelectorCommand implements IExtensionSingleActivationService, IDisposable {
     public readonly supportedWorkspaceTypes = { untrustedWorkspace: false, virtualWorkspace: true };
@@ -21,6 +23,8 @@ export abstract class BaseInterpreterSelectorCommand implements IExtensionSingle
         @unmanaged() protected readonly commandManager: ICommandManager,
         @unmanaged() protected readonly applicationShell: IApplicationShell,
         @unmanaged() protected readonly workspaceService: IWorkspaceService,
+        @unmanaged() protected readonly pathUtils: IPathUtils,
+        @unmanaged() protected readonly configurationService: IConfigurationService,
     ) {
         this.disposables.push(this);
     }
@@ -31,52 +35,85 @@ export abstract class BaseInterpreterSelectorCommand implements IExtensionSingle
 
     public abstract activate(): Promise<void>;
 
-    protected async getConfigTarget(): Promise<
+    protected async getConfigTargets(options?: {
+        resetTarget?: boolean;
+    }): Promise<
         | {
               folderUri: Resource;
               configTarget: ConfigurationTarget;
-          }
+          }[]
         | undefined
     > {
-        if (
-            !Array.isArray(this.workspaceService.workspaceFolders) ||
-            this.workspaceService.workspaceFolders.length === 0
-        ) {
-            return {
-                folderUri: undefined,
-                configTarget: ConfigurationTarget.Global,
-            };
+        const workspaceFolders = this.workspaceService.workspaceFolders;
+        if (workspaceFolders === undefined || workspaceFolders.length === 0) {
+            return [
+                {
+                    folderUri: undefined,
+                    configTarget: ConfigurationTarget.Global,
+                },
+            ];
         }
-        if (!this.workspaceService.workspaceFile && this.workspaceService.workspaceFolders.length === 1) {
-            return {
-                folderUri: this.workspaceService.workspaceFolders[0].uri,
-                configTarget: ConfigurationTarget.WorkspaceFolder,
-            };
+        if (!this.workspaceService.workspaceFile && workspaceFolders.length === 1) {
+            return [
+                {
+                    folderUri: workspaceFolders[0].uri,
+                    configTarget: ConfigurationTarget.WorkspaceFolder,
+                },
+            ];
         }
 
         // Ok we have multiple workspaces, get the user to pick a folder.
 
-        type WorkspaceSelectionQuickPickItem = QuickPickItem & { uri: Uri };
-        const quickPickItems: WorkspaceSelectionQuickPickItem[] = [
-            ...this.workspaceService.workspaceFolders.map((w) => ({
-                label: w.name,
-                description: path.dirname(w.uri.fsPath),
-                uri: w.uri,
-            })),
+        let quickPickItems: WorkspaceSelectionQuickPickItem[] = options?.resetTarget
+            ? [
+                  {
+                      label: Common.clearAll(),
+                  },
+              ]
+            : [];
+        quickPickItems.push(
+            ...workspaceFolders.map((w) => {
+                const selectedInterpreter = this.pathUtils.getDisplayName(
+                    this.configurationService.getSettings(w.uri).pythonPath,
+                    w.uri.fsPath,
+                );
+                return {
+                    label: w.name,
+                    description: this.pathUtils.getDisplayName(path.dirname(w.uri.fsPath)),
+                    uri: w.uri,
+                    detail: selectedInterpreter,
+                };
+            }),
             {
-                label: Interpreters.entireWorkspace(),
-                uri: this.workspaceService.workspaceFolders[0].uri,
+                label: options?.resetTarget ? Interpreters.clearAtWorkspace() : Interpreters.entireWorkspace(),
+                uri: workspaceFolders[0].uri,
             },
-        ];
+        );
 
         const selection = await this.applicationShell.showQuickPick(quickPickItems, {
-            placeHolder: 'Select the workspace to set the interpreter',
+            placeHolder: options?.resetTarget
+                ? 'Select the workspace folder to clear the interpreter for'
+                : 'Select the workspace folder to set the interpreter',
         });
 
+        if (selection?.label === Common.clearAll()) {
+            const folderTargets: {
+                folderUri: Resource;
+                configTarget: ConfigurationTarget;
+            }[] = workspaceFolders.map((w) => ({
+                folderUri: w.uri,
+                configTarget: ConfigurationTarget.WorkspaceFolder,
+            }));
+            return [
+                ...folderTargets,
+                { folderUri: workspaceFolders[0].uri, configTarget: ConfigurationTarget.Workspace },
+            ];
+        }
+
         return selection
-            ? selection.label === Interpreters.entireWorkspace()
-                ? { folderUri: selection.uri, configTarget: ConfigurationTarget.Workspace }
-                : { folderUri: selection.uri, configTarget: ConfigurationTarget.WorkspaceFolder }
+            ? selection.label === Interpreters.entireWorkspace() || selection.label === Interpreters.clearAtWorkspace()
+                ? [{ folderUri: selection.uri, configTarget: ConfigurationTarget.Workspace }]
+                : [{ folderUri: selection.uri, configTarget: ConfigurationTarget.WorkspaceFolder }]
             : undefined;
     }
 }
diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/resetInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/resetInterpreter.ts
@@ -6,6 +6,7 @@
 import { inject, injectable } from 'inversify';
 import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../common/application/types';
 import { Commands } from '../../../../common/constants';
+import { IConfigurationService, IPathUtils } from '../../../../common/types';
 import { IPythonPathUpdaterServiceManager } from '../../types';
 import { BaseInterpreterSelectorCommand } from './base';
 
@@ -16,8 +17,17 @@ export class ResetInterpreterCommand extends BaseInterpreterSelectorCommand {
         @inject(ICommandManager) commandManager: ICommandManager,
         @inject(IApplicationShell) applicationShell: IApplicationShell,
         @inject(IWorkspaceService) workspaceService: IWorkspaceService,
+        @inject(IPathUtils) pathUtils: IPathUtils,
+        @inject(IConfigurationService) configurationService: IConfigurationService,
     ) {
-        super(pythonPathUpdaterService, commandManager, applicationShell, workspaceService);
+        super(
+            pythonPathUpdaterService,
+            commandManager,
+            applicationShell,
+            workspaceService,
+            pathUtils,
+            configurationService,
+        );
     }
 
     public async activate() {
@@ -27,13 +37,16 @@ export class ResetInterpreterCommand extends BaseInterpreterSelectorCommand {
     }
 
     public async resetInterpreter() {
-        const targetConfig = await this.getConfigTarget();
-        if (!targetConfig) {
+        const targetConfigs = await this.getConfigTargets({ resetTarget: true });
+        if (!targetConfigs) {
             return;
         }
-        const configTarget = targetConfig.configTarget;
-        const wkspace = targetConfig.folderUri;
-
-        await this.pythonPathUpdaterService.updatePythonPath(undefined, configTarget, 'ui', wkspace);
+        await Promise.all(
+            targetConfigs.map(async (targetConfig) => {
+                const configTarget = targetConfig.configTarget;
+                const wkspace = targetConfig.folderUri;
+                await this.pythonPathUpdaterService.updatePythonPath(undefined, configTarget, 'ui', wkspace);
+            }),
+        );
     }
 }
diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts
@@ -55,18 +55,25 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand {
 
     constructor(
         @inject(IApplicationShell) applicationShell: IApplicationShell,
-        @inject(IPathUtils) private readonly pathUtils: IPathUtils,
+        @inject(IPathUtils) pathUtils: IPathUtils,
         @inject(IPythonPathUpdaterServiceManager)
         pythonPathUpdaterService: IPythonPathUpdaterServiceManager,
-        @inject(IConfigurationService) private readonly configurationService: IConfigurationService,
+        @inject(IConfigurationService) configurationService: IConfigurationService,
         @inject(ICommandManager) commandManager: ICommandManager,
         @inject(IMultiStepInputFactory) private readonly multiStepFactory: IMultiStepInputFactory,
         @inject(IPlatformService) private readonly platformService: IPlatformService,
         @inject(IInterpreterSelector) private readonly interpreterSelector: IInterpreterSelector,
         @inject(IWorkspaceService) workspaceService: IWorkspaceService,
         @inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
     ) {
-        super(pythonPathUpdaterService, commandManager, applicationShell, workspaceService);
+        super(
+            pythonPathUpdaterService,
+            commandManager,
+            applicationShell,
+            workspaceService,
+            pathUtils,
+            configurationService,
+        );
     }
 
     public async activate(): Promise<void> {
@@ -302,13 +309,13 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand {
 
     @captureTelemetry(EventName.SELECT_INTERPRETER)
     public async setInterpreter(): Promise<void> {
-        const targetConfig = await this.getConfigTarget();
+        const targetConfig = await this.getConfigTargets();
         if (!targetConfig) {
             return;
         }
 
-        const { configTarget } = targetConfig;
-        const wkspace = targetConfig.folderUri;
+        const { configTarget } = targetConfig[0];
+        const wkspace = targetConfig[0].folderUri;
         const interpreterState: InterpreterStateArgs = { path: undefined, workspace: wkspace };
         const multiStep = this.multiStepFactory.create<InterpreterStateArgs>();
         await multiStep.run((input, s) => this._pickInterpreter(input, s), interpreterState);
diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setShebangInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setShebangInterpreter.ts
@@ -5,29 +5,25 @@
 
 import { inject, injectable } from 'inversify';
 import { ConfigurationTarget } from 'vscode';
-import {
-    IApplicationShell,
-    ICommandManager,
-    IDocumentManager,
-    IWorkspaceService,
-} from '../../../../common/application/types';
+import { IExtensionSingleActivationService } from '../../../../activation/types';
+import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../../../common/application/types';
 import { Commands } from '../../../../common/constants';
+import { IDisposableRegistry } from '../../../../common/types';
 import { IShebangCodeLensProvider } from '../../../contracts';
 import { IPythonPathUpdaterServiceManager } from '../../types';
-import { BaseInterpreterSelectorCommand } from './base';
 
 @injectable()
-export class SetShebangInterpreterCommand extends BaseInterpreterSelectorCommand {
+export class SetShebangInterpreterCommand implements IExtensionSingleActivationService {
+    public readonly supportedWorkspaceTypes = { untrustedWorkspace: false, virtualWorkspace: true };
     constructor(
-        @inject(IWorkspaceService) workspaceService: IWorkspaceService,
+        @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
         @inject(IDocumentManager) private readonly documentManager: IDocumentManager,
-        @inject(IPythonPathUpdaterServiceManager) pythonPathUpdaterService: IPythonPathUpdaterServiceManager,
+        @inject(IPythonPathUpdaterServiceManager)
+        private readonly pythonPathUpdaterService: IPythonPathUpdaterServiceManager,
         @inject(IShebangCodeLensProvider) private readonly shebangCodeLensProvider: IShebangCodeLensProvider,
-        @inject(ICommandManager) commandManager: ICommandManager,
-        @inject(ICommandManager) applicationShell: IApplicationShell,
-    ) {
-        super(pythonPathUpdaterService, commandManager, applicationShell, workspaceService);
-    }
+        @inject(ICommandManager) private readonly commandManager: ICommandManager,
+        @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry,
+    ) {}
 
     public async activate() {
         this.disposables.push(
diff --git a/src/test/configuration/interpreterSelector/commands/resetInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/resetInterpreter.unit.test.ts
diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Add an option to clear interpreter setting for all workspace folders in multiroot scenario.`