Change the logic for when we show the "Install Python (Extension)" commands

news/1 Enhancements/10583.md

@@ -0,0 +1 @@
+Change the logic to show our "Install Python (Extension)" commands in the kernel picker more often.

src/notebooks/controllers/installPythonControllerCommands.ts

@@ -6,20 +6,28 @@ import {
     NotebookCell,
     NotebookCellExecutionState,
     NotebookCellExecutionStateChangeEvent,
+    NotebookEditor,
     notebooks,
     window
 } from 'vscode';
 import { isPythonKernelConnection } from '../../kernels/helpers';
 import { IExtensionSingleActivationService } from '../../platform/activation/types';
 import { IPythonApiProvider, IPythonExtensionChecker } from '../../platform/api/types';
 import { IApplicationShell, ICommandManager } from '../../platform/common/application/types';
-import { Commands, JupyterNotebookView, PythonExtension, Telemetry } from '../../platform/common/constants';
+import {
+    Commands,
+    JupyterNotebookView,
+    PythonExtension,
+    PYTHON_LANGUAGE,
+    Telemetry
+} from '../../platform/common/constants';
 import { ContextKey } from '../../platform/common/contextKey';
 import { IDisposableRegistry, IsWebExtension } from '../../platform/common/types';
 import { Common, DataScience } from '../../platform/common/utils/localize';
 import { traceError, traceInfo } from '../../platform/logging';
 import { ProgressReporter } from '../../platform/progress/progressReporter';
 import { sendTelemetryEvent } from '../../telemetry';
+import { getLanguageOfNotebookDocument } from '../languages/helpers';
 import { IControllerLoader, IControllerRegistration } from './types';
 
 // This service owns the commands that show up in the kernel picker to allow for either installing
@@ -30,6 +38,7 @@ export class InstallPythonControllerCommands implements IExtensionSingleActivati
     private showInstallPythonContext: ContextKey;
     // WeakSet of executing cells, so they get cleaned up on document close without worrying
     private executingCells: WeakSet<NotebookCell> = new WeakSet<NotebookCell>();
+    private foundPythonConnections: boolean = false;
     constructor(
         @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry,
         @inject(ICommandManager) private readonly commandManager: ICommandManager,
@@ -70,6 +79,9 @@ export class InstallPythonControllerCommands implements IExtensionSingleActivati
 
         // We need to know when controllers have been updated so that we can update our context keys
         this.disposables.push(this.controllerLoader.refreshed(this.onNotebookControllersLoaded, this));
+
+        // Also track active notebook editor change
+        this.disposables.push(window.onDidChangeActiveNotebookEditor(this.onDidChangeActiveNotebookEditor, this));
     }
 
     // Track if there are any cells currently executing or pending
@@ -86,26 +98,41 @@ export class InstallPythonControllerCommands implements IExtensionSingleActivati
         }
     }
 
-    // When the manager loads new controllers we need to check and see if we should enable or disable our context
-    // keys that control our commands
-    private async onNotebookControllersLoaded() {
-        if (!this.isWeb) {
-            if (this.controllerRegistration.values.some((item) => isPythonKernelConnection(item.connection))) {
-                // We have some type of python kernel, turn off both install helper commands
-                await this.showInstallPythonExtensionContext.set(false);
-                await this.showInstallPythonContext.set(false);
-            } else {
+    private async onDidChangeActiveNotebookEditor(editor: NotebookEditor | undefined) {
+        if (!this.isWeb && editor) {
+            // Make sure we are only showing these for python notebooks or undefined notebooks
+            const lang = getLanguageOfNotebookDocument(editor.notebook);
+            if (!lang || lang === PYTHON_LANGUAGE) {
                 if (!this.extensionChecker.isPythonExtensionInstalled) {
-                    // If we don't have the extension installed, show extension install command
+                    // Python or undefined notebook with no extension, recommend installing extension
                     await this.showInstallPythonExtensionContext.set(true);
                     await this.showInstallPythonContext.set(false);
-                } else {
-                    // If we do have the extension installed, show python install command
+                    return;
+                }
+
+                if (!this.foundPythonConnections) {
+                    // Extension is installed, but we didn't find any python connections
+                    // recommend installing python in this case
                     await this.showInstallPythonExtensionContext.set(false);
                     await this.showInstallPythonContext.set(true);
+                    return;
                 }
             }
         }
+
+        // Final fallback is to always hide the commands
+        await this.showInstallPythonExtensionContext.set(false);
+        await this.showInstallPythonContext.set(false);
+    }
+
+    // Check if we actually found python connections after loading controllers
+    private async onNotebookControllersLoaded() {
+        this.foundPythonConnections = this.controllerRegistration.values.some((item) =>
+            isPythonKernelConnection(item.connection)
+        );
+
+        // If we just finished loading, make sure to check the active document
+        await this.onDidChangeActiveNotebookEditor(window.activeNotebookEditor);
     }
 
     // This is called via the "install python" command in the kernel picker in the case where

src/notebooks/languages/cellLanguageService.ts

@@ -5,7 +5,7 @@
 
 import type * as nbformat from '@jupyterlab/nbformat';
 import { inject, injectable, named } from 'inversify';
-import { Memento, NotebookCellKind, NotebookDocument } from 'vscode';
+import { Memento, NotebookDocument } from 'vscode';
 import { IExtensionSingleActivationService } from '../../platform/activation/types';
 import { IPythonExtensionChecker } from '../../platform/api/types';
 import { IVSCodeNotebook } from '../../platform/common/application/types';
@@ -14,16 +14,12 @@ import {
     PYTHON_LANGUAGE,
     VSCodeKnownNotebookLanguages
 } from '../../platform/common/constants';
-import { traceWarning } from '../../platform/logging';
 import { IDisposableRegistry, IMemento, GLOBAL_MEMENTO } from '../../platform/common/types';
 import { swallowExceptions } from '../../platform/common/utils/decorators';
-import {
-    getKernelConnectionLanguage,
-    getLanguageInNotebookMetadata,
-    isPythonKernelConnection
-} from '../../kernels/helpers';
-import { getNotebookMetadata, isJupyterNotebook, translateKernelLanguageToMonaco } from '../../platform/common/utils';
+import { getKernelConnectionLanguage, isPythonKernelConnection } from '../../kernels/helpers';
+import { isJupyterNotebook, translateKernelLanguageToMonaco } from '../../platform/common/utils';
 import { IJupyterKernelSpec, KernelConnectionMetadata } from '../../kernels/types';
+import { getLanguageOfNotebookDocument } from './helpers';
 
 export const LastSavedNotebookCellLanguage = 'DATASCIENCE.LAST_SAVED_CELL_LANGUAGE';
 /**
@@ -79,21 +75,9 @@ export class NotebookCellLanguageService implements IExtensionSingleActivationSe
         if (!isJupyterNotebook(doc)) {
             return;
         }
-        const language = this.getLanguageOfFirstCodeCell(doc);
+        const language = getLanguageOfNotebookDocument(doc);
         if (language && language !== this.lastSavedNotebookCellLanguage) {
             await this.globalMemento.update(LastSavedNotebookCellLanguage, language);
         }
     }
-    private getLanguageOfFirstCodeCell(doc: NotebookDocument) {
-        // If the document has been closed, accessing cell information can fail.
-        // Ignore such exceptions.
-        try {
-            // Give preference to the language information in the metadata.
-            const language = getLanguageInNotebookMetadata(getNotebookMetadata(doc));
-            // Fall back to the language of the first code cell in the notebook.
-            return language || doc.getCells().find((cell) => cell.kind === NotebookCellKind.Code)?.document.languageId;
-        } catch (ex) {
-            traceWarning('Failed to determine language of first cell', ex);
-        }
-    }
 }

src/notebooks/languages/helpers.ts

@@ -0,0 +1,22 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import { NotebookCellKind, NotebookDocument } from 'vscode';
+import { getLanguageInNotebookMetadata } from '../../kernels/helpers';
+import { getNotebookMetadata } from '../../platform/common/utils';
+import { traceWarning } from '../../platform/logging';
+
+// Get the language of the notebook document, preference given to metadata over the language of
+// the first cell
+export function getLanguageOfNotebookDocument(doc: NotebookDocument): string | undefined {
+    // If the document has been closed, accessing cell information can fail.
+    // Ignore such exceptions.
+    try {
+        // Give preference to the language information in the metadata.
+        const language = getLanguageInNotebookMetadata(getNotebookMetadata(doc));
+        // Fall back to the language of the first code cell in the notebook.
+        return language || doc.getCells().find((cell) => cell.kind === NotebookCellKind.Code)?.document.languageId;
+    } catch (ex) {
+        traceWarning('Failed to determine language of first cell', ex);
+    }
+}