Changes to kernel picker (#9113)

DonJayamanne · web-flow · commit 7cf7ec4f60ac · 2019-12-16T08:33:15.000-08:00
* Changes to display of kernel picker
* Fix tests
* Fixes
diff --git a/src/client/datascience/datascience.ts b/src/client/datascience/datascience.ts
@@ -22,7 +22,7 @@ import { hasCells } from './cellFactory';
 import { Commands, EditorContexts, Settings, Telemetry } from './constants';
 import { createRemoteConnectionInfo } from './jupyter/jupyterUtils';
 import { KernelSelector, KernelSpecInterpreter } from './jupyter/kernels/kernelSelector';
-import { ICodeWatcher, IConnection, IDataScience, IDataScienceCodeLensProvider, IDataScienceCommandListener, IJupyterSessionManagerFactory, INotebookEditorProvider } from './types';
+import { ICodeWatcher, IConnection, IDataScience, IDataScienceCodeLensProvider, IDataScienceCommandListener, IJupyterKernel, IJupyterKernelSpec, IJupyterSessionManagerFactory, INotebookEditorProvider } from './types';
 
 interface ISelectUriQuickPickItem extends vscode.QuickPickItem {
     newChoice: boolean;
@@ -227,14 +227,14 @@ export class DataScience implements IDataScience {
     }
 
     @captureTelemetry(Telemetry.SelectLocalJupyterKernel)
-    public async selectLocalJupyterKernel(): Promise<KernelSpecInterpreter> {
-        return this.kernelSelector.selectLocalKernel();
+    public async selectLocalJupyterKernel(currentKernel?: IJupyterKernelSpec | IJupyterKernel & Partial<IJupyterKernelSpec>): Promise<KernelSpecInterpreter> {
+        return this.kernelSelector.selectLocalKernel(undefined, undefined, currentKernel);
     }
 
     @captureTelemetry(Telemetry.SelectRemoteJupyuterKernel)
-    public async selectRemoteJupyterKernel(connInfo: IConnection): Promise<KernelSpecInterpreter> {
+    public async selectRemoteJupyterKernel(connInfo: IConnection, currentKernel?: IJupyterKernelSpec | IJupyterKernel & Partial<IJupyterKernelSpec>): Promise<KernelSpecInterpreter> {
         const session = await this.jupyterSessionManagerFactory.create(connInfo);
-        return this.kernelSelector.selectRemoteKernel(session);
+        return this.kernelSelector.selectRemoteKernel(session, undefined, currentKernel);
     }
 
     public async debugCell(file: string, startLine: number, startChar: number, endLine: number, endChar: number): Promise<void> {
diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts
@@ -1290,12 +1290,12 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
         let kernel: KernelSpecInterpreter | undefined;
 
         if (settings.datascience.jupyterServerURI.toLowerCase() === Settings.JupyterServerLocalLaunch) {
-            kernel = await this.dataScience.selectLocalJupyterKernel();
+            kernel = await this.dataScience.selectLocalJupyterKernel(this.notebook?.getKernelSpec());
         } else if (this.notebook) {
             const connInfo = this.notebook.server.getConnectionInfo();
-
+            const currentKernel = this.notebook.getKernelSpec();
             if (connInfo) {
-                kernel = await this.dataScience.selectRemoteJupyterKernel(connInfo);
+                kernel = await this.dataScience.selectRemoteJupyterKernel(connInfo, currentKernel);
             }
         }
 
diff --git a/src/client/datascience/jupyter/jupyterServer.ts b/src/client/datascience/jupyter/jupyterServer.ts
@@ -60,8 +60,7 @@ export class JupyterServerBase implements INotebookServer {
         // Try creating a session just to ensure we're connected. Callers of this function check to make sure jupyter
         // is running and connectable.
         let session: IJupyterSession | undefined;
-        // tslint:disable-next-line: no-any
-        session = await this.sessionManager.startNew(launchInfo.kernelSpec as any, cancelToken);
+        session = await this.sessionManager.startNew(launchInfo.kernelSpec, cancelToken);
         const idleTimeout = this.configService.getSettings().datascience.jupyterLaunchTimeout;
         // The wait for idle should throw if we can't connect.
         await session.waitForIdle(idleTimeout);
diff --git a/src/client/datascience/jupyter/kernels/kernelSelections.ts b/src/client/datascience/jupyter/kernels/kernelSelections.ts
@@ -7,6 +7,7 @@ import { inject, injectable } from 'inversify';
 import { CancellationToken } from 'vscode';
 import { PYTHON_LANGUAGE } from '../../../common/constants';
 import { IFileSystem } from '../../../common/platform/types';
+import { IPathUtils } from '../../../common/types';
 import * as localize from '../../../common/utils/localize';
 import { IInterpreterSelector } from '../../../interpreter/configuration/types';
 import { IJupyterKernel, IJupyterKernelSpec, IJupyterSessionManager } from '../../types';
@@ -22,9 +23,11 @@ import { IKernelSelectionListProvider, IKernelSpecQuickPickItem } from './types'
  * @param {IJupyterKernelSpec} kernelSpec
  * @returns {IKernelSpecQuickPickItem}
  */
-function getQuickPickItemForKernelSpec(kernelSpec: IJupyterKernelSpec): IKernelSpecQuickPickItem {
+function getQuickPickItemForKernelSpec(kernelSpec: IJupyterKernelSpec, pathUtils: IPathUtils): IKernelSpecQuickPickItem {
     return {
         label: kernelSpec.display_name,
+        // If we have a matching interpreter, then display that path in the dropdown else path of the kernelspec.
+        detail: pathUtils.getDisplayName(kernelSpec.metadata?.interpreter?.path || kernelSpec.path),
         selection: { kernelModel: undefined, kernelSpec: kernelSpec, interpreter: undefined }
     };
 }
@@ -35,9 +38,12 @@ function getQuickPickItemForKernelSpec(kernelSpec: IJupyterKernelSpec): IKernelS
  * @param {(IJupyterKernel & Partial<IJupyterKernelSpec>)} kernel
  * @returns {IKernelSpecQuickPickItem}
  */
-function getQuickPickItemForActiveKernel(kernel: IJupyterKernel & Partial<IJupyterKernelSpec>): IKernelSpecQuickPickItem {
+function getQuickPickItemForActiveKernel(kernel: IJupyterKernel & Partial<IJupyterKernelSpec>, pathUtils: IPathUtils): IKernelSpecQuickPickItem {
+    const path = kernel.metadata?.interpreter?.path || kernel.path;
     return {
         label: kernel.display_name || kernel.name || '',
+        // If we have a matching interpreter, then display that path in the dropdown else path of the kernelspec.
+        detail: path ? pathUtils.getDisplayName(path) : '',
         description: localize.DataScience.jupyterSelectURIRunningDetailFormat().format(kernel.lastActivityTime.toLocaleString(), kernel.numberOfConnections.toString()),
         selection: { kernelModel: kernel, kernelSpec: undefined, interpreter: undefined }
     };
@@ -51,7 +57,7 @@ function getQuickPickItemForActiveKernel(kernel: IJupyterKernel & Partial<IJupyt
  * @implements {IKernelSelectionListProvider}
  */
 export class ActiveJupyterSessionKernelSelectionListProvider implements IKernelSelectionListProvider {
-    constructor(private readonly sessionManager: IJupyterSessionManager) {}
+    constructor(private readonly sessionManager: IJupyterSessionManager, private readonly pathUtils: IPathUtils) {}
     public async getKernelSelections(_cancelToken?: CancellationToken | undefined): Promise<IKernelSpecQuickPickItem[]> {
         const [activeKernels, kernelSpecs] = await Promise.all([this.sessionManager.getRunningKernels(), this.sessionManager.getKernelSpecs()]);
         const items = activeKernels.map(item => {
@@ -64,7 +70,7 @@ export class ActiveJupyterSessionKernelSelectionListProvider implements IKernelS
         return items
             .filter(item => item.display_name || item.name)
             .filter(item => (item.language || '').toLowerCase() === PYTHON_LANGUAGE.toLowerCase())
-            .map(getQuickPickItemForActiveKernel);
+            .map(item => getQuickPickItemForActiveKernel(item, this.pathUtils));
     }
 }
 
@@ -76,13 +82,13 @@ export class ActiveJupyterSessionKernelSelectionListProvider implements IKernelS
  * @implements {IKernelSelectionListProvider}
  */
 export class InstalledJupyterKernelSelectionListProvider implements IKernelSelectionListProvider {
-    constructor(private readonly kernelService: KernelService, private readonly sessionManager?: IJupyterSessionManager) {}
+    constructor(private readonly kernelService: KernelService, private readonly pathUtils: IPathUtils, private readonly sessionManager?: IJupyterSessionManager) {}
     public async getKernelSelections(cancelToken?: CancellationToken | undefined): Promise<IKernelSpecQuickPickItem[]> {
         const items = await this.kernelService.getKernelSpecs(this.sessionManager, cancelToken);
         return items
             .filter(item => (item.language || '').toLowerCase() === PYTHON_LANGUAGE.toLowerCase())
-            .map(getQuickPickItemForKernelSpec);
-    }
+            .map(item => getQuickPickItemForKernelSpec(item, this.pathUtils));
+        }
 }
 
 /**
@@ -100,8 +106,7 @@ export class InterpreterKernelSelectionListProvider implements IKernelSelectionL
         return items.map(item => {
             return {
                 ...item,
-                // We don't want details & descriptions.
-                detail: '',
+                // We don't want descriptions.
                 description: '',
                 selection: { kernelModel: undefined, interpreter: item.interpreter, kernelSpec: undefined }
             };
@@ -122,7 +127,8 @@ export class KernelSelectionProvider {
     constructor(
         @inject(KernelService) private readonly kernelService: KernelService,
         @inject(IInterpreterSelector) private readonly interpreterSelector: IInterpreterSelector,
-        @inject(IFileSystem) private readonly fileSystem: IFileSystem) {}
+        @inject(IFileSystem) private readonly fileSystem: IFileSystem,
+        @inject(IPathUtils) private readonly pathUtils: IPathUtils) {}
     /**
      * Gets a selection of kernel specs from a remote session.
      *
@@ -132,7 +138,7 @@ export class KernelSelectionProvider {
      * @memberof KernelSelectionProvider
      */
     public async getKernelSelectionsForRemoteSession(sessionManager: IJupyterSessionManager, cancelToken?: CancellationToken): Promise<IKernelSpecQuickPickItem[]> {
-        const liveItems = new ActiveJupyterSessionKernelSelectionListProvider(sessionManager).getKernelSelections(cancelToken).then(items => {
+        const liveItems = new ActiveJupyterSessionKernelSelectionListProvider(sessionManager, this.pathUtils).getKernelSelections(cancelToken).then(items => {
             // Sorty by name.
             items.sort((a, b) => a.label === b.label ? 0 : (a.label > b.label ? 1 : -1));
             this.remoteSuggestionsCache = items;
@@ -153,7 +159,7 @@ export class KernelSelectionProvider {
      */
     public async getKernelSelectionsForLocalSession(sessionManager?: IJupyterSessionManager, cancelToken?: CancellationToken): Promise<IKernelSpecQuickPickItem[]> {
         const getSelections = async () => {
-            const installedKernelsPromise = new InstalledJupyterKernelSelectionListProvider(this.kernelService, sessionManager).getKernelSelections(cancelToken);
+            const installedKernelsPromise = new InstalledJupyterKernelSelectionListProvider(this.kernelService, this.pathUtils, sessionManager).getKernelSelections(cancelToken);
             const interpretersPromise = new InterpreterKernelSelectionListProvider(this.interpreterSelector).getKernelSelections(cancelToken);
 
             // tslint:disable-next-line: prefer-const
@@ -168,8 +174,8 @@ export class KernelSelectionProvider {
                 }
                 return true;
             }).map(item => {
-                // We don't want details & descriptions.
-                return {...item, detail: '', description: ''};
+                // We don't want descriptions.
+                return {...item, description: ''};
             });
 
             const unifiedList = [...installedKernels!, ...interpreters];
diff --git a/src/client/datascience/jupyter/kernels/kernelSelector.ts b/src/client/datascience/jupyter/kernels/kernelSelector.ts
@@ -56,9 +56,9 @@ export class KernelSelector {
      * @returns {Promise<KernelSpecInterpreter>}
      * @memberof KernelSelector
      */
-    public async selectRemoteKernel(session: IJupyterSessionManager, cancelToken?: CancellationToken): Promise<KernelSpecInterpreter> {
+    public async selectRemoteKernel(session: IJupyterSessionManager, cancelToken?: CancellationToken, currentKernel?: IJupyterKernelSpec | IJupyterKernel & Partial<IJupyterKernelSpec>): Promise<KernelSpecInterpreter> {
         const suggestions = await this.selectionProvider.getKernelSelectionsForRemoteSession(session, cancelToken);
-        return this.selectKernel(suggestions, session, cancelToken);
+        return this.selectKernel(suggestions, session, cancelToken, currentKernel);
     }
     /**
      * Select a kernel from a local session.
@@ -68,9 +68,9 @@ export class KernelSelector {
      * @returns {Promise<KernelSpecInterpreter>}
      * @memberof KernelSelector
      */
-    public async selectLocalKernel(session?: IJupyterSessionManager, cancelToken?: CancellationToken): Promise<KernelSpecInterpreter> {
+    public async selectLocalKernel(session?: IJupyterSessionManager, cancelToken?: CancellationToken, currentKernel?: IJupyterKernelSpec | IJupyterKernel & Partial<IJupyterKernelSpec>): Promise<KernelSpecInterpreter> {
         const suggestions = await this.selectionProvider.getKernelSelectionsForLocalSession(session, cancelToken);
-        return this.selectKernel(suggestions, session, cancelToken);
+        return this.selectKernel(suggestions, session, cancelToken, currentKernel);
     }
     /**
      * Gets a kernel that needs to be used with a local session.
@@ -120,8 +120,9 @@ export class KernelSelector {
         }
         return selection;
     }
-    private async selectKernel(suggestions: IKernelSpecQuickPickItem[], session?: IJupyterSessionManager, cancelToken?: CancellationToken){
-        const selection = await this.applicationShell.showQuickPick(suggestions, { placeHolder: localize.DataScience.selectKernel() }, cancelToken);
+    private async selectKernel(suggestions: IKernelSpecQuickPickItem[], session?: IJupyterSessionManager, cancelToken?: CancellationToken, currentKernel?: IJupyterKernelSpec | IJupyterKernel & Partial<IJupyterKernelSpec>){
+        const placeHolder = localize.DataScience.selectKernel() + (currentKernel ? ` (current: ${currentKernel.display_name || currentKernel.name})` : '');
+        const selection = await this.applicationShell.showQuickPick(suggestions, { placeHolder }, cancelToken);
         if (!selection?.selection) {
             return {};
         }
@@ -132,7 +133,7 @@ export class KernelSelector {
         } else if (selection.selection.kernelModel) {
             sendTelemetryEvent(Telemetry.SwitchToExistingKernel);
             // tslint:disable-next-line: no-any
-            const interpreter = selection.selection.kernelModel ? await this.kernelService.findMatchingInterpreter(selection.selection.kernelModel as any, cancelToken) : undefined;
+            const interpreter = selection.selection.kernelModel ? await this.kernelService.findMatchingInterpreter(selection.selection.kernelModel, cancelToken) : undefined;
             return { kernelSpec: selection.selection.kernelSpec, interpreter, kernelModel: selection.selection.kernelModel };
         } else if (selection.selection.kernelSpec) {
             sendTelemetryEvent(Telemetry.SwitchToExistingKernel);
diff --git a/src/client/datascience/jupyter/kernels/kernelService.ts b/src/client/datascience/jupyter/kernels/kernelService.ts
@@ -22,7 +22,7 @@ import { IEnvironmentActivationService } from '../../../interpreter/activation/t
 import { IInterpreterService, PythonInterpreter } from '../../../interpreter/contracts';
 import { captureTelemetry, sendTelemetryEvent } from '../../../telemetry';
 import { JupyterCommands, Telemetry } from '../../constants';
-import { IJupyterKernelSpec, IJupyterSessionManager } from '../../types';
+import { IJupyterKernel, IJupyterKernelSpec, IJupyterSessionManager } from '../../types';
 import { JupyterCommandFinder } from '../jupyterCommandFinder';
 import { JupyterKernelSpec } from './jupyterKernelSpec';
 
@@ -116,8 +116,8 @@ export class KernelService {
      * @returns {(Promise<PythonInterpreter | undefined>)}
      * @memberof KernelService
      */
-    public async findMatchingInterpreter(kernelSpec: IJupyterKernelSpec, cancelToken?: CancellationToken): Promise<PythonInterpreter | undefined> {
-        if (kernelSpec.language.toLowerCase() !== PYTHON_LANGUAGE) {
+    public async findMatchingInterpreter(kernelSpec: IJupyterKernelSpec | IJupyterKernel & Partial<IJupyterKernelSpec>, cancelToken?: CancellationToken): Promise<PythonInterpreter | undefined> {
+        if (kernelSpec.language && kernelSpec.language?.toLowerCase() !== PYTHON_LANGUAGE) {
             return;
         }
         const activeInterpreterPromise = this.interpreterService.getActiveInterpreter(undefined);
diff --git a/src/test/datascience/jupyter/kernels/kernelSelections.unit.test.ts b/src/test/datascience/jupyter/kernels/kernelSelections.unit.test.ts
diff --git a/src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts b/src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts

Original file line number	Diff line number	Diff line change
`@@ -1290,12 +1290,12 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp`
`1290`	`1290`	`let kernel: KernelSpecInterpreter \| undefined;`
`1291`	`1291`
`1292`	`1292`	`if (settings.datascience.jupyterServerURI.toLowerCase() === Settings.JupyterServerLocalLaunch) {`
`1293`		`- kernel = await this.dataScience.selectLocalJupyterKernel();`
	`1293`	`+ kernel = await this.dataScience.selectLocalJupyterKernel(this.notebook?.getKernelSpec());`
`1294`	`1294`	`} else if (this.notebook) {`
`1295`	`1295`	`const connInfo = this.notebook.server.getConnectionInfo();`
`1296`		`-`
	`1296`	`+ const currentKernel = this.notebook.getKernelSpec();`
`1297`	`1297`	`if (connInfo) {`
`1298`		`- kernel = await this.dataScience.selectRemoteJupyterKernel(connInfo);`
	`1298`	`+ kernel = await this.dataScience.selectRemoteJupyterKernel(connInfo, currentKernel);`
`1299`	`1299`	`}`
`1300`	`1300`	`}`
`1301`	`1301`