diff --git a/src/vs/platform/actions/common/actions.ts b/src/vs/platform/actions/common/actions.ts index 934694a50656..ac227d339b56 100644 --- a/src/vs/platform/actions/common/actions.ts +++ b/src/vs/platform/actions/common/actions.ts @@ -78,6 +78,7 @@ export class MenuId { static readonly EditorContextPeek = new MenuId('EditorContextPeek'); static readonly EditorContextShare = new MenuId('EditorContextShare'); // --- Start Positron --- + static readonly PositronNotebookKernelSubmenu = new MenuId('PositronNotebookKernelSubmenu'); static readonly EditorActionsLeft = new MenuId('EditorActionsLeft'); static readonly EditorActionsRight = new MenuId('EditorActionsRight'); // --- End Positron --- diff --git a/src/vs/platform/positronActionBar/browser/components/actionBarButton.tsx b/src/vs/platform/positronActionBar/browser/components/actionBarButton.tsx index c043bcd08e1e..e016b2bdf334 100644 --- a/src/vs/platform/positronActionBar/browser/components/actionBarButton.tsx +++ b/src/vs/platform/positronActionBar/browser/components/actionBarButton.tsx @@ -174,6 +174,7 @@ export const ActionBarButton = forwardRef< {props.label} } + {props.children} {props.dropdownIndicator === 'enabled' &&
@@ -205,7 +206,6 @@ export const ActionBarButton = forwardRef< onPressed={props.onPressed} > - {props.children} ); } else { @@ -242,7 +242,6 @@ export const ActionBarButton = forwardRef< >
- {props.children}
); } diff --git a/src/vs/platform/positronActionBar/browser/components/actionBarMenuButton.tsx b/src/vs/platform/positronActionBar/browser/components/actionBarMenuButton.tsx index 7dd001a40be0..9951ef431aad 100644 --- a/src/vs/platform/positronActionBar/browser/components/actionBarMenuButton.tsx +++ b/src/vs/platform/positronActionBar/browser/components/actionBarMenuButton.tsx @@ -7,7 +7,7 @@ import './actionBarMenuButton.css'; // React. -import React, { useEffect, useRef, useState } from 'react'; +import React, { PropsWithChildren, useEffect, useRef, useState } from 'react'; // Other dependencies. import { ActionBarButton } from './actionBarButton.js'; @@ -48,7 +48,7 @@ interface ActionBarMenuButtonProps { * @param props An ActionBarMenuButtonProps that contains the component properties. * @returns The rendered component. */ -export const ActionBarMenuButton = (props: ActionBarMenuButtonProps) => { +export const ActionBarMenuButton = (props: PropsWithChildren) => { // Context hooks. const services = usePositronReactServicesContext(); const positronActionBarContext = usePositronActionBarContext(); @@ -74,15 +74,16 @@ export const ActionBarMenuButton = (props: ActionBarMenuButtonProps) => { } }, [positronActionBarContext.menuShowing]); + const { actions: getActions } = props; const getMenuActions = React.useCallback(async () => { - const actions = await props.actions(); + const actions = await getActions(); const defaultAction = actions.find(action => action.checked); setDefaultAction(defaultAction); setActions(actions); return actions; - }, [props]); + }, [getActions]); useEffect(() => { getMenuActions(); diff --git a/src/vs/platform/positronActionBar/browser/components/actionBarWidget.css b/src/vs/platform/positronActionBar/browser/components/actionBarWidget.css index 1a9972a4a7ec..bb290b0f8af3 100644 --- a/src/vs/platform/positronActionBar/browser/components/actionBarWidget.css +++ b/src/vs/platform/positronActionBar/browser/components/actionBarWidget.css @@ -3,10 +3,6 @@ * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. *--------------------------------------------------------------------------------------------*/ -.action-bar-widget { - padding-inline: 6px; -} - /** * Error indicator shown when a widget fails to render. * Displays a small error icon with tooltip explaining the failure. diff --git a/src/vs/workbench/contrib/positronConsole/browser/components/consoleInstanceState.tsx b/src/vs/workbench/contrib/positronConsole/browser/components/consoleInstanceState.tsx index e3190a94d546..2aaa73877f2a 100644 --- a/src/vs/workbench/contrib/positronConsole/browser/components/consoleInstanceState.tsx +++ b/src/vs/workbench/contrib/positronConsole/browser/components/consoleInstanceState.tsx @@ -12,28 +12,17 @@ import React, { useEffect, useState } from 'react'; // Other dependencies. import { IPositronConsoleInstance, PositronConsoleState } from '../../../../services/positronConsole/browser/interfaces/positronConsoleService.js'; import { DisposableStore } from '../../../../../base/common/lifecycle.js'; - -const enum StatusIconClassName { - ACTIVE = 'codicon-positron-status-active', - DISCONNECTED = 'codicon-positron-status-disconnected', - IDLE = 'codicon-positron-status-idle' -} - -const statusIconClassNameToColor = { - [StatusIconClassName.ACTIVE]: 'var(--vscode-positronConsole-stateIconActive)', - [StatusIconClassName.DISCONNECTED]: 'var(--vscode-positronConsole-stateIconDisconnected)', - [StatusIconClassName.IDLE]: 'var(--vscode-positronConsole-stateIconIdle)' -} - -const consoleStateToStatusIcon = { - [PositronConsoleState.Uninitialized]: StatusIconClassName.DISCONNECTED, - [PositronConsoleState.Disconnected]: StatusIconClassName.DISCONNECTED, - [PositronConsoleState.Starting]: StatusIconClassName.ACTIVE, - [PositronConsoleState.Busy]: StatusIconClassName.ACTIVE, - [PositronConsoleState.Ready]: StatusIconClassName.IDLE, - [PositronConsoleState.Offline]: StatusIconClassName.DISCONNECTED, - [PositronConsoleState.Exiting]: StatusIconClassName.ACTIVE, - [PositronConsoleState.Exited]: StatusIconClassName.DISCONNECTED +import { RuntimeStatus, RuntimeStatusIcon } from './runtimeStatus.js'; + +const consoleStateToRuntimeStatus = { + [PositronConsoleState.Uninitialized]: RuntimeStatus.Disconnected, + [PositronConsoleState.Disconnected]: RuntimeStatus.Disconnected, + [PositronConsoleState.Starting]: RuntimeStatus.Active, + [PositronConsoleState.Busy]: RuntimeStatus.Active, + [PositronConsoleState.Ready]: RuntimeStatus.Idle, + [PositronConsoleState.Offline]: RuntimeStatus.Disconnected, + [PositronConsoleState.Exiting]: RuntimeStatus.Active, + [PositronConsoleState.Exited]: RuntimeStatus.Disconnected }; interface ConsoleInstanceStateProps { @@ -55,13 +44,9 @@ export const ConsoleInstanceState = ({ positronConsoleInstance }: ConsoleInstanc return () => disposableStore.dispose(); }, [positronConsoleInstance]); - const icon = consoleStateToStatusIcon[consoleState]; - const color = statusIconClassNameToColor[icon]; + const runtimeStatus = consoleStateToRuntimeStatus[consoleState]; return ( - + ); } diff --git a/src/vs/workbench/contrib/positronConsole/browser/components/consoleTabList.tsx b/src/vs/workbench/contrib/positronConsole/browser/components/consoleTabList.tsx index d17b1da44a08..c116f02c1c21 100644 --- a/src/vs/workbench/contrib/positronConsole/browser/components/consoleTabList.tsx +++ b/src/vs/workbench/contrib/positronConsole/browser/components/consoleTabList.tsx @@ -22,6 +22,7 @@ import { PositronConsoleTabFocused } from '../../../../common/contextkeys.js'; import { usePositronReactServicesContext } from '../../../../../base/browser/positronReactRendererContext.js'; import { LanguageRuntimeSessionMode } from '../../../../services/languageRuntime/common/languageRuntimeService.js'; import { basename } from '../../../../../base/common/path.js'; +import { RuntimeIcon } from './runtimeIcon.js'; /** * The minimum width required for the delete action to be displayed on the console tab. @@ -378,16 +379,10 @@ const ConsoleTab = ({ positronConsoleInstance, width, onChangeSession }: Console onMouseDown={handleMouseDown} > - { - !isNotebookSession && - - } - {isNotebookSession && - - } + {isRenamingSession ? ( { + const classNames = ['icon'] + if (sessionMode === LanguageRuntimeSessionMode.Notebook) { + classNames.push(...ThemeIcon.asClassNameArray(Codicon.notebook)); + return ; + } + if (base64EncodedIconSvg === undefined) { + return null; + } + return ; +}; diff --git a/src/vs/workbench/contrib/positronConsole/browser/components/runtimeStatus.css b/src/vs/workbench/contrib/positronConsole/browser/components/runtimeStatus.css new file mode 100644 index 000000000000..d4d2fcf77d0a --- /dev/null +++ b/src/vs/workbench/contrib/positronConsole/browser/components/runtimeStatus.css @@ -0,0 +1,4 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2025 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ diff --git a/src/vs/workbench/contrib/positronConsole/browser/components/runtimeStatus.tsx b/src/vs/workbench/contrib/positronConsole/browser/components/runtimeStatus.tsx new file mode 100644 index 000000000000..cd69b36ae2dc --- /dev/null +++ b/src/vs/workbench/contrib/positronConsole/browser/components/runtimeStatus.tsx @@ -0,0 +1,71 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2025 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +// CSS. +import './runtimeStatus.css'; + +// React. +import React from 'react'; + +// Other dependencies. +import { POSITRON_CONSOLE_STATE_ICON_ACTIVE, POSITRON_CONSOLE_STATE_ICON_DISCONNECTED, POSITRON_CONSOLE_STATE_ICON_IDLE } from '../../../../common/theme.js'; +import { ThemeIcon } from '../../../../../base/common/themables.js'; +import { Codicon } from '../../../../../base/common/codicons.js'; +import { registerIcon } from '../../../../../platform/theme/common/iconRegistry.js'; +import { localize } from '../../../../../nls.js'; +import { asCssVariable, ColorIdentifier } from '../../../../../platform/theme/common/colorUtils.js'; + +export const enum RuntimeStatus { + Active = 'Active', + Disconnected = 'Disconnected', + Idle = 'Idle' +} + +const positronRuntimeStatusActiveIcon = registerIcon( + 'positron-runtime-status-active', + Codicon.positronStatusActive, + localize('positronRuntimeStatusActiveIcon', 'Icon to indicate the \'active\' status of an interpreter session.') +); + +const positronRuntimeStatusDisconnectedIcon = registerIcon( + 'positron-runtime-status-disconnected', + Codicon.positronStatusDisconnected, + localize('positronRuntimeStatusDisconnectedIcon', 'Icon to indicate the \'disconnected\' status of an interpreter session.') +); + +const positronRuntimeStatusIdleIcon = registerIcon( + 'positron-runtime-status-idle', + Codicon.positronStatusIdle, + localize('positronRuntimeStatusIdleIcon', 'Icon to indicate the \'idle\' status of an interpreter session.') +); + +const statusToIcon: Record = { + [RuntimeStatus.Active]: positronRuntimeStatusActiveIcon, + [RuntimeStatus.Disconnected]: positronRuntimeStatusDisconnectedIcon, + [RuntimeStatus.Idle]: positronRuntimeStatusIdleIcon, +}; + +const statusToIconColor: Record = { + [RuntimeStatus.Active]: POSITRON_CONSOLE_STATE_ICON_ACTIVE, + [RuntimeStatus.Disconnected]: POSITRON_CONSOLE_STATE_ICON_DISCONNECTED, + [RuntimeStatus.Idle]: POSITRON_CONSOLE_STATE_ICON_IDLE, +}; + +export interface RuntimeStatusIconProps { + status: RuntimeStatus; +} + +export const RuntimeStatusIcon = ({ status }: RuntimeStatusIconProps) => { + const icon = statusToIcon[status]; + const className = ThemeIcon.asClassName(icon); + const color = statusToIconColor[status]; + const colorCss = asCssVariable(color); + return ( + + ); +}; diff --git a/src/vs/workbench/contrib/positronNotebook/browser/IPositronNotebookInstance.ts b/src/vs/workbench/contrib/positronNotebook/browser/IPositronNotebookInstance.ts index 807f25991098..93224571afe2 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/IPositronNotebookInstance.ts +++ b/src/vs/workbench/contrib/positronNotebook/browser/IPositronNotebookInstance.ts @@ -7,12 +7,13 @@ import { IObservable } from '../../../../base/common/observable.js'; import { URI } from '../../../../base/common/uri.js'; import { CellKind, IPositronNotebookCell } from './PositronNotebookCells/IPositronNotebookCell.js'; import { SelectionStateMachine } from './selectionMachine.js'; -import { ILanguageRuntimeSession } from '../../../services/runtimeSession/common/runtimeSessionService.js'; +import { INotebookLanguageRuntimeSession } from '../../../services/runtimeSession/common/runtimeSessionService.js'; import { Event } from '../../../../base/common/event.js'; import { ICodeEditor } from '../../../../editor/browser/editorBrowser.js'; import { IBaseCellEditorOptions } from '../../notebook/browser/notebookBrowser.js'; import { NotebookOptions } from '../../notebook/browser/notebookOptions.js'; import { PositronNotebookContextKeyManager } from './ContextKeysManager.js'; +import { IScopedContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; /** * Represents the possible states of a notebook's kernel connection */ @@ -54,6 +55,8 @@ export interface IPositronNotebookInstance { */ get uri(): URI; + readonly scopedContextKeyService: IScopedContextKeyService | undefined; + /** * Indicates whether this notebook instance is currently connected to a view/editor. * Used to determine if the notebook is currently being displayed. @@ -88,7 +91,7 @@ export interface IPositronNotebookInstance { * Observable reference to the current runtime session for the notebook. * This manages the connection to the kernel and execution environment. */ - readonly runtimeSession: IObservable; + readonly runtimeSession: IObservable; /** * State machine that manages cell selection behavior and state. diff --git a/src/vs/workbench/contrib/positronNotebook/browser/KernelStatusBadge.css b/src/vs/workbench/contrib/positronNotebook/browser/KernelStatusBadge.css index 506f7f9f6603..9236ef27b6c7 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/KernelStatusBadge.css +++ b/src/vs/workbench/contrib/positronNotebook/browser/KernelStatusBadge.css @@ -10,24 +10,22 @@ overflow: hidden; text-overflow: ellipsis; max-width: 200px; + margin-right: 4px; - & span.kernel-status { - font-weight: bold; - } - - & .Uninitialized { - color: grey; - } + /* Use this property to take padding into account when sizing (prevents overflow) */ + box-sizing: border-box; + display: flex; + align-items: center; + column-gap: 4px; - & .Connected { - color: forestgreen; - } - - & .Disconnected { - color: red; - } + .runtime-name { + /* Grow items after session name to fill available space */ + flex: 1; - & .Connecting { - color: orange; + margin: 0; + line-height: 22px; + overflow: hidden; + min-width: 0; + text-overflow: ellipsis; } } diff --git a/src/vs/workbench/contrib/positronNotebook/browser/KernelStatusBadge.tsx b/src/vs/workbench/contrib/positronNotebook/browser/KernelStatusBadge.tsx index 472aaa319583..70b7e6f704dc 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/KernelStatusBadge.tsx +++ b/src/vs/workbench/contrib/positronNotebook/browser/KernelStatusBadge.tsx @@ -12,21 +12,68 @@ import React from 'react'; // Other dependencies. import { useNotebookInstance } from './NotebookInstanceProvider.js'; import { useObservedValue } from './useObservedValue.js'; +import { KernelStatus } from './IPositronNotebookInstance.js'; +import { RuntimeStatus, RuntimeStatusIcon } from '../../positronConsole/browser/components/runtimeStatus.js'; +import { localize } from '../../../../nls.js'; +import { MenuId, MenuItemAction, SubmenuItemAction } from '../../../../platform/actions/common/actions.js'; +import { ActionBarMenuButton } from '../../../../platform/positronActionBar/browser/components/actionBarMenuButton.js'; +import { useMenu } from './useMenu.js'; +import { IPositronNotebookActionBarContext } from '../../runtimeNotebookKernel/browser/runtimeNotebookKernelActions.js'; + +const kernelStatusToRuntimeStatus = { + [KernelStatus.Uninitialized]: RuntimeStatus.Disconnected, + [KernelStatus.Disconnected]: RuntimeStatus.Disconnected, + [KernelStatus.Connected]: RuntimeStatus.Idle, + [KernelStatus.Connecting]: RuntimeStatus.Active, + [KernelStatus.Errored]: RuntimeStatus.Disconnected, +}; + +const tooltip = localize('positronNotebook.kernelStatusBadge.tooltip', 'Kernel Actions'); +const noRuntimeLabel = localize('positronNotebook.kernelStatusBadge.noRuntimeLabel', 'No Kernel Selected'); -/** - * KernelStatusBadge - A presentational component that displays the current kernel status. - * - * The widget simply displays the kernel status using the notebook instance from context. - * All interaction logic is handled by the action bar widget system. - */ export function KernelStatusBadge() { + // Context const notebookInstance = useNotebookInstance(); - const kernelStatus = useObservedValue(notebookInstance.kernelStatus); + + // State + const runtimeStatus = useObservedValue( + notebookInstance.kernelStatus.map((kernelStatus) => kernelStatusToRuntimeStatus[kernelStatus]) + ); + const runtimeName = useObservedValue( + notebookInstance.runtimeSession.map((runtimeSession) => + runtimeSession ? runtimeSession.runtimeMetadata.runtimeName : noRuntimeLabel) + ); + const menu = useMenu(MenuId.PositronNotebookKernelSubmenu, notebookInstance.scopedContextKeyService); + + // Callback to load actions from the menu + const getActions = React.useCallback(() => { + if (!menu.current) { + return []; + } + const actions: (MenuItemAction | SubmenuItemAction)[] = []; + for (const [_group, groupActions] of menu.current.getActions({ + arg: { + uri: notebookInstance.uri, + ui: true, + } satisfies IPositronNotebookActionBarContext, + shouldForwardArgs: true, + })) { + actions.push(...groupActions); + } + return actions; + }, [menu, notebookInstance.uri]); return ( -
- Kernel - {' ' + kernelStatus} -
+ +
+ +

{runtimeName}

+
+
); } diff --git a/src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookInstance.ts b/src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookInstance.ts index 6e9143de21bc..200430dd5eb2 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookInstance.ts +++ b/src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookInstance.ts @@ -28,7 +28,7 @@ import { IPositronNotebookService } from './positronNotebookService.js'; import { IPositronNotebookInstance, KernelStatus } from './IPositronNotebookInstance.js'; import { NotebookCellTextModel } from '../../notebook/common/model/notebookCellTextModel.js'; import { ICommandService } from '../../../../platform/commands/common/commands.js'; -import { SELECT_KERNEL_ID_POSITRON, SelectPositronNotebookKernelContext } from './SelectPositronNotebookKernelAction.js'; +import { SELECT_KERNEL_ID_POSITRON } from './SelectPositronNotebookKernelAction.js'; import { INotebookKernelService } from '../../notebook/common/notebookKernelService.js'; import { IRuntimeSessionService } from '../../../services/runtimeSession/common/runtimeSessionService.js'; import { RuntimeState } from '../../../services/languageRuntime/common/languageRuntimeService.js'; @@ -40,6 +40,7 @@ import { ICodeEditor } from '../../../../editor/browser/editorBrowser.js'; import { cellToCellDto2, serializeCellsToClipboard } from './cellClipboardUtils.js'; import { IClipboardService } from '../../../../platform/clipboard/common/clipboardService.js'; import { IPositronConsoleService } from '../../../services/positronConsole/browser/interfaces/positronConsoleService.js'; +import { isNotebookLanguageRuntimeSession } from '../../../services/runtimeSession/common/runtimeSession.js'; interface IPositronNotebookInstanceRequiredTextModel extends IPositronNotebookInstance { textModel: NotebookTextModel; @@ -138,6 +139,8 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot */ private _container: HTMLElement | undefined = undefined; + private _scopedContextKeyService: IScopedContextKeyService | undefined; + /** * The DOM element that contains the cells for the notebook. */ @@ -214,6 +217,10 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot return this._cellsContainer; } + get scopedContextKeyService(): IScopedContextKeyService | undefined { + return this._scopedContextKeyService; + } + /** * Sets the DOM element that contains the cells for the notebook. * @param container The container element to set, or null to clear @@ -350,7 +357,7 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot // Track the current runtime session for this notebook this.runtimeSession = observableValue('positronNotebookRuntimeSession', this.runtimeSessionService.getNotebookSessionForNotebookUri(this.uri)); this._register(this.runtimeSessionService.onDidStartRuntime((session) => { - if (session.metadata.notebookUri && this._isThisNotebook(session.metadata.notebookUri)) { + if (isNotebookLanguageRuntimeSession(session) && this._isThisNotebook(session.metadata.notebookUri)) { this.runtimeSession.set(session, undefined); } })); @@ -793,6 +800,7 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot async attachView(container: HTMLElement, scopedContextKeyService: IScopedContextKeyService) { this.detachView(); this._container = container; + this._scopedContextKeyService = scopedContextKeyService; this.contextManager.setContainer(container, scopedContextKeyService); this._logService.info(this.id, 'attachView'); @@ -952,10 +960,7 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot if (this.kernelStatus.get() !== KernelStatus.Connected) { this._logService.info(this.id, 'No kernel connected, attempting to connect'); // Attempt to connect to the kernel - await this._commandService.executeCommand( - SELECT_KERNEL_ID_POSITRON, - { forceDropdown: false } satisfies SelectPositronNotebookKernelContext - ); + await this._commandService.executeCommand(SELECT_KERNEL_ID_POSITRON); } const hasExecutions = [...cells].some(cell => Boolean(this.notebookExecutionStateService.getCellExecution(cell.uri))); diff --git a/src/vs/workbench/contrib/positronNotebook/browser/SelectPositronNotebookKernelAction.ts b/src/vs/workbench/contrib/positronNotebook/browser/SelectPositronNotebookKernelAction.ts index d260702e861f..a7e6d0f1515f 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/SelectPositronNotebookKernelAction.ts +++ b/src/vs/workbench/contrib/positronNotebook/browser/SelectPositronNotebookKernelAction.ts @@ -4,40 +4,42 @@ *--------------------------------------------------------------------------------------------*/ import { localize2, localize } from '../../../../nls.js'; -import { Action2, registerAction2 } from '../../../../platform/actions/common/actions.js'; +import { Action2, MenuId, registerAction2 } from '../../../../platform/actions/common/actions.js'; import { ContextKeyExpr } from '../../../../platform/contextkey/common/contextkey.js'; import { ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js'; import { IQuickInputService, IQuickPickItem } from '../../../../platform/quickinput/common/quickInput.js'; -import { selectKernelIcon } from '../../notebook/browser/notebookIcons.js'; import { INotebookKernelService, INotebookKernel } from '../../notebook/common/notebookKernelService.js'; import { PositronNotebookInstance } from './PositronNotebookInstance.js'; import { POSITRON_RUNTIME_NOTEBOOK_KERNELS_EXTENSION_ID } from '../../runtimeNotebookKernel/common/runtimeNotebookKernelConfig.js'; import { IEditorService } from '../../../services/editor/common/editorService.js'; import { getNotebookInstanceFromEditorPane } from './notebookUtils.js'; +import { Codicon } from '../../../../base/common/codicons.js'; +import { IPositronNotebookActionBarContext } from '../../runtimeNotebookKernel/browser/runtimeNotebookKernelActions.js'; export const SELECT_KERNEL_ID_POSITRON = 'positronNotebook.selectKernel'; const NOTEBOOK_ACTIONS_CATEGORY_POSITRON = localize2('positronNotebookActions.category', 'Positron Notebook'); const NOTEBOOK_IS_ACTIVE_EDITOR = ContextKeyExpr.equals('activeEditor', 'workbench.editor.positronNotebook'); -export interface SelectPositronNotebookKernelContext { - forceDropdown: boolean; -} - class SelectPositronNotebookKernelAction extends Action2 { constructor() { super({ id: SELECT_KERNEL_ID_POSITRON, category: NOTEBOOK_ACTIONS_CATEGORY_POSITRON, - title: localize2('positronNotebookActions.selectKernel', 'Select Positron Notebook Kernel'), - icon: selectKernelIcon, + title: localize2('positronNotebookActions.changeKernel', 'Change Kernel...'), + icon: Codicon.gear, f1: true, precondition: NOTEBOOK_IS_ACTIVE_EDITOR, + menu: [{ + id: MenuId.PositronNotebookKernelSubmenu, + order: 0, + }] }); } - async run(accessor: ServicesAccessor, context?: SelectPositronNotebookKernelContext): Promise { - const { forceDropdown } = context || { forceDropdown: false }; + async run(accessor: ServicesAccessor, context?: IPositronNotebookActionBarContext): Promise { + // Force the dropdown if the action was invoked by the user in the UI + const forceDropdown = context?.ui ?? false; const notebookKernelService = accessor.get(INotebookKernelService); const activeNotebook = getNotebookInstanceFromEditorPane(accessor.get(IEditorService)); const quickInputService = accessor.get(IQuickInputService); diff --git a/src/vs/workbench/contrib/positronNotebook/browser/positronNotebook.contribution.ts b/src/vs/workbench/contrib/positronNotebook/browser/positronNotebook.contribution.ts index 1193d63bd7ed..031ac92f5c08 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/positronNotebook.contribution.ts +++ b/src/vs/workbench/contrib/positronNotebook/browser/positronNotebook.contribution.ts @@ -821,7 +821,7 @@ registerNotebookAction({ icon: ThemeIcon.fromId('notebook-execute-all'), positronActionBarOptions: { controlType: 'button', - displayTitle: true + displayTitle: false } }, keybinding: { @@ -841,7 +841,7 @@ registerNotebookAction({ icon: ThemeIcon.fromId('positron-clean'), positronActionBarOptions: { controlType: 'button', - displayTitle: true + displayTitle: false } }, keybinding: { @@ -854,10 +854,9 @@ registerNotebookAction({ commandId: 'positronNotebook.showConsole', handler: (notebook) => notebook.showNotebookConsole(), menu: { - id: MenuId.EditorActionsLeft, - group: 'navigation', - order: 30, - title: { value: localize('showConsole', 'Show Console'), original: 'Show Console' }, + id: MenuId.PositronNotebookKernelSubmenu, + order: 100, + title: { value: localize('showConsole', 'Open Notebook Console'), original: 'Open Notebook Console' }, icon: ThemeIcon.fromId('terminal'), positronActionBarOptions: { controlType: 'button', @@ -874,9 +873,9 @@ registerNotebookAction({ notebook.addCell(CellKind.Code, cellCount); }, menu: { - id: MenuId.EditorActionsRight, + id: MenuId.EditorActionsLeft, group: 'navigation', - order: 10, + order: 30, title: { value: localize('addCodeCell', 'Code'), original: 'Code' }, icon: ThemeIcon.fromId('add'), positronActionBarOptions: { @@ -894,9 +893,9 @@ registerNotebookAction({ notebook.addCell(CellKind.Markup, cellCount); }, menu: { - id: MenuId.EditorActionsRight, + id: MenuId.EditorActionsLeft, group: 'navigation', - order: 20, + order: 40, title: { value: localize('addMarkdownCell', 'Markdown'), original: 'Markdown' }, icon: ThemeIcon.fromId('add'), positronActionBarOptions: { @@ -907,15 +906,12 @@ registerNotebookAction({ }); // Kernel Status Widget - Shows live kernel connection status at far right of action bar -// Widget is command-driven: clicking opens the kernel picker dropdown +// Widget is self-contained: manages its own menu interactions via ActionBarMenuButton registerNotebookAction({ id: 'positronNotebook.kernelStatus', widget: { component: KernelStatusBadge, - commandId: 'positronNotebook.selectKernel', - commandArgs: { forceDropdown: true }, - ariaLabel: 'Select notebook kernel', - tooltip: 'Click to select a kernel' + selfContained: true }, menu: { id: MenuId.EditorActionsRight, diff --git a/src/vs/workbench/contrib/positronNotebook/browser/registerNotebookAction.tsx b/src/vs/workbench/contrib/positronNotebook/browser/registerNotebookAction.tsx index b27b38737967..9ee026262edc 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/registerNotebookAction.tsx +++ b/src/vs/workbench/contrib/positronNotebook/browser/registerNotebookAction.tsx @@ -290,7 +290,7 @@ function registerNotebookCommandInternal(options: INotebookCommandOptions): IDis icon: options.menu.icon, positronActionBarOptions: options.menu.positronActionBarOptions }, - group: options.menu.group ?? 'navigation', + group: options.menu.group, order: options.menu.order, when: menuWhen }); diff --git a/src/vs/workbench/contrib/positronNotebook/browser/useDisposableStore.tsx b/src/vs/workbench/contrib/positronNotebook/browser/useDisposableStore.tsx deleted file mode 100644 index 30bbbb5831dc..000000000000 --- a/src/vs/workbench/contrib/positronNotebook/browser/useDisposableStore.tsx +++ /dev/null @@ -1,23 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (C) 2024 Posit Software, PBC. All rights reserved. - * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. - *--------------------------------------------------------------------------------------------*/ - -// React. -import React from 'react'; - -// Other dependencies. -import { DisposableStore } from '../../../../base/common/lifecycle.js'; - -/** - * Create a disposable store for the component. - * Will cleanup/dispose stored disposables when the component is unmounted. - */ -export function useDisposableStore(): DisposableStore { - const disposables = React.useRef(new DisposableStore()); - React.useEffect(() => { - const current = disposables.current; - return () => current.dispose(); - }, []); - return disposables.current; -} diff --git a/src/vs/workbench/contrib/positronNotebook/browser/useMenu.tsx b/src/vs/workbench/contrib/positronNotebook/browser/useMenu.tsx new file mode 100644 index 000000000000..226b065b2ef4 --- /dev/null +++ b/src/vs/workbench/contrib/positronNotebook/browser/useMenu.tsx @@ -0,0 +1,48 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2025 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ +import React from 'react'; +import { usePositronReactServicesContext } from '../../../../base/browser/positronReactRendererContext.js'; +import { combinedDisposable } from '../../../../base/common/lifecycle.js'; +import { MenuId, IMenuCreateOptions, IMenuService, IMenu } from '../../../../platform/actions/common/actions.js'; +import { IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; + +/** + * React wrapper for an IMenu. + * + * @param menuId The menu's identifier + * @param contextKeyService The context key service used to evaluate menu item visibility and enablement + * @param options Optional configuration for menu creation + * @returns An object containing the current menu instance and a version number that increments on menu changes + */ +export function useMenu( + menuId: MenuId, + contextKeyService: IContextKeyService | undefined, + options?: IMenuCreateOptions) { + // Context + const menuService = usePositronReactServicesContext().get(IMenuService); + + // State + const [menu, setMenu] = React.useState(); + const [version, setVersion] = React.useState(0); + + // Main effect + React.useEffect(() => { + if (!contextKeyService) { + return; + } + + const menu = menuService.createMenu(menuId, contextKeyService, options); + setMenu(menu); + setVersion(0); + + const disposable = combinedDisposable( + menu, + menu.onDidChange(() => setVersion(v => v + 1)), + ); + return () => disposable.dispose(); + }, [menuService, contextKeyService, menuId, options]); + + return { current: menu, version }; +} diff --git a/src/vs/workbench/contrib/positronNotebook/browser/useObservedValue.tsx b/src/vs/workbench/contrib/positronNotebook/browser/useObservedValue.tsx index 0f7759877a87..f6c98afee270 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/useObservedValue.tsx +++ b/src/vs/workbench/contrib/positronNotebook/browser/useObservedValue.tsx @@ -7,7 +7,7 @@ import React from 'react'; // Other dependencies. -import { autorun, IObservable } from '../../../../base/common/observable.js'; +import { IObservable, runOnChange } from '../../../../base/common/observable.js'; /** * Automatically updates the component when the observable changes. @@ -18,13 +18,8 @@ export function useObservedValue(observable: IObservable): T { const [value, setValue] = React.useState(observable.get()); React.useEffect(() => { - const disposable = autorun(reader => { - const val = observable.read(reader); - setValue(val); - }); - return () => { - disposable.dispose(); - }; + const disposable = runOnChange(observable, setValue); + return () => disposable.dispose(); }, [observable]); return value; diff --git a/src/vs/workbench/contrib/runtimeNotebookKernel/browser/runtimeNotebookKernelActions.ts b/src/vs/workbench/contrib/runtimeNotebookKernel/browser/runtimeNotebookKernelActions.ts index 7225e05f217b..1b7fdc69d509 100644 --- a/src/vs/workbench/contrib/runtimeNotebookKernel/browser/runtimeNotebookKernelActions.ts +++ b/src/vs/workbench/contrib/runtimeNotebookKernel/browser/runtimeNotebookKernelActions.ts @@ -4,18 +4,22 @@ *--------------------------------------------------------------------------------------------*/ import { Codicon } from '../../../../base/common/codicons.js'; +import { KeyChord, KeyCode } from '../../../../base/common/keyCodes.js'; +import { isUriComponents, URI } from '../../../../base/common/uri.js'; import { localize, localize2 } from '../../../../nls.js'; import { Action2, MenuId, registerAction2 } from '../../../../platform/actions/common/actions.js'; import { ContextKeyExpr } from '../../../../platform/contextkey/common/contextkey.js'; import { ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js'; +import { KeybindingWeight } from '../../../../platform/keybinding/common/keybindingsRegistry.js'; import { INotificationService } from '../../../../platform/notification/common/notification.js'; import { IProgressService, ProgressLocation } from '../../../../platform/progress/common/progress.js'; import { IEditorService } from '../../../services/editor/common/editorService.js'; -import { IRuntimeSessionService } from '../../../services/runtimeSession/common/runtimeSessionService.js'; +import { RuntimeExitReason } from '../../../services/languageRuntime/common/languageRuntimeService.js'; +import { INotebookLanguageRuntimeSession, IRuntimeSessionService } from '../../../services/runtimeSession/common/runtimeSessionService.js'; import { NotebookEditorWidget } from '../../notebook/browser/notebookEditorWidget.js'; import { NOTEBOOK_KERNEL } from '../../notebook/common/notebookContextKeys.js'; -import { isNotebookEditorInput } from '../../notebook/common/notebookEditorInput.js'; -import { ActiveNotebookHasRunningRuntime } from '../common/activeRuntimeNotebookContextManager.js'; +import { POSITRON_NOTEBOOK_EDITOR_CONTAINER_FOCUSED } from '../../positronNotebook/browser/ContextKeysManager.js'; +import { ActiveNotebookHasRunningRuntime, isNotebookEditorInput } from '../common/activeRuntimeNotebookContextManager.js'; import { POSITRON_RUNTIME_NOTEBOOK_KERNELS_EXTENSION_ID } from '../common/runtimeNotebookKernelConfig.js'; const category = localize2('positron.runtimeNotebookKernel.category', "Notebook"); @@ -23,79 +27,192 @@ const category = localize2('positron.runtimeNotebookKernel.category', "Notebook" /** Whether a Positron kernel is selected for the active notebook. */ const NOTEBOOK_POSITRON_KERNEL_SELECTED = ContextKeyExpr.regex(NOTEBOOK_KERNEL.key, new RegExp(`${POSITRON_RUNTIME_NOTEBOOK_KERNELS_EXTENSION_ID}\/.*`)); -/** The context for actions run from the notebook editor toolbar. */ +/** The context for actions run from the VSCode notebook editor toolbar. */ interface INotebookEditorToolbarContext { ui: boolean; notebookEditor: NotebookEditorWidget; source: 'notebookToolbar'; } +export interface IPositronNotebookActionBarContext { + ui: boolean; + uri: URI; +} + +function isPositronNotebookActionBarContext(obj: unknown): obj is IPositronNotebookActionBarContext { + const context = obj as IPositronNotebookActionBarContext; + return !!context && typeof context.ui === 'boolean' && isUriComponents(context.uri); +} + +/** The context for actions in a notebook using a language runtime kernel. */ +interface IRuntimeNotebookKernelActionContext { + /** The notebook's language runtime session, if any */ + runtimeSession: INotebookLanguageRuntimeSession | undefined; + source: { + /** The source of the action */ + id: 'positronNotebookActionBar' | 'vscodeNotebookToolbar' | 'command'; + /** Debug message noting the source of the action */ + debugMessage: string; + }; +} + +abstract class BaseRuntimeNotebookKernelAction extends Action2 { + abstract runWithContext(accessor: ServicesAccessor, context?: IRuntimeNotebookKernelActionContext): Promise; + + override async run(accessor: ServicesAccessor, context?: INotebookEditorToolbarContext | IPositronNotebookActionBarContext): Promise { + const editorService = accessor.get(IEditorService); + const runtimeSessionService = accessor.get(IRuntimeSessionService); + + // Try to use the notebook URI from the context - set if run via the notebook editor toolbar. + let notebookUri: URI | undefined; + let source: IRuntimeNotebookKernelActionContext['source']; + if (context) { + if (isPositronNotebookActionBarContext(context)) { + source = { + id: 'positronNotebookActionBar', + debugMessage: 'User clicked restart button in Positron notebook editor toolbar', + }; + notebookUri = context.uri; + } else { + source = { + id: 'vscodeNotebookToolbar', + debugMessage: 'User clicked restart button in VSCode notebook editor toolbar' + }; + notebookUri = context.notebookEditor.textModel?.uri; + } + } else { + source = { + id: 'command', + debugMessage: `Restart notebook kernel command ${RuntimeNotebookKernelRestartAction.ID} executed`, + }; + const activeEditor = editorService.activeEditor; + if (!isNotebookEditorInput(activeEditor)) { + throw new Error('No active notebook. This command should only be available when a notebook is active.'); + } + notebookUri = activeEditor.resource; + } + + const runtimeSession = notebookUri && runtimeSessionService.getNotebookSessionForNotebookUri(notebookUri); + + return this.runWithContext(accessor, { runtimeSession, source }); + } +} + /** Restart the active runtime notebook kernel. */ -class RuntimeNotebookKernelRestartAction extends Action2 { - /** The action's ID. */ +export class RuntimeNotebookKernelRestartAction extends BaseRuntimeNotebookKernelAction { public static readonly ID = 'positron.runtimeNotebookKernel.restart'; constructor() { super({ id: RuntimeNotebookKernelRestartAction.ID, title: localize2('positron.command.restartNotebookInterpreter', 'Restart Kernel'), - icon: Codicon.debugRestart, + positronActionBarOptions: { + controlType: 'button', + displayTitle: false + }, + icon: Codicon.positronRestartRuntimeThin, f1: true, category, precondition: ActiveNotebookHasRunningRuntime, + keybinding: { + weight: KeybindingWeight.WorkbenchContrib, + primary: KeyChord(KeyCode.Digit0, KeyCode.Digit0), + when: POSITRON_NOTEBOOK_EDITOR_CONTAINER_FOCUSED, + }, menu: [ + // VSCode notebooks { id: MenuId.NotebookToolbar, group: 'navigation/execute@5', order: 5, when: NOTEBOOK_POSITRON_KERNEL_SELECTED, + }, + // Positron notebooks + { + id: MenuId.PositronNotebookKernelSubmenu, + order: 10, } ] }); } - override async run(accessor: ServicesAccessor, context?: INotebookEditorToolbarContext): Promise { - const editorService = accessor.get(IEditorService); + override async runWithContext(accessor: ServicesAccessor, context?: IRuntimeNotebookKernelActionContext): Promise { + const session = context?.runtimeSession; + if (!session) { + throw new Error('No session found for active notebook. This command should only be available when a session is running.'); + } + const progressService = accessor.get(IProgressService); const notificationService = accessor.get(INotificationService); const runtimeSessionService = accessor.get(IRuntimeSessionService); - // Try to use the notebook URI from the context - set if run via the notebook editor toolbar. - let notebookUri = context?.notebookEditor.textModel?.uri; - - // If no context was provided, try to get the active notebook URI. - if (!notebookUri) { - const activeEditor = editorService.activeEditor; - if (!isNotebookEditorInput(activeEditor)) { - throw new Error('No active notebook. This command should only be available when a notebook is active.'); - } - notebookUri = activeEditor.resource; + // Restart the session with a progress bar. + try { + await progressService.withProgress({ + location: ProgressLocation.Notification, + title: localize("positron.notebook.restart.restarting", "Restarting {0} interpreter for '{1}'", + session.runtimeMetadata.runtimeName, session.metadata.notebookUri.fsPath), + }, () => runtimeSessionService.restartSession(session.metadata.sessionId, context.source.debugMessage)); + } catch (error) { + notificationService.error( + localize("positron.notebook.restart.failed", "Restarting {0} interpreter for '{1}' failed. Reason: {2}", + session.runtimeMetadata.runtimeName, session.metadata.notebookUri.fsPath, error.message)); } + } +} - // Get the session for the active notebook. - const session = runtimeSessionService.getNotebookSessionForNotebookUri(notebookUri); +/** Shutdown the active runtime notebook kernel. */ +export class RuntimeNotebookKernelShutdownAction extends BaseRuntimeNotebookKernelAction { + public static readonly ID = 'positron.runtimeNotebookKernel.shutdown'; + + constructor() { + super({ + id: RuntimeNotebookKernelShutdownAction.ID, + title: localize2('positron.command.shutdownNotebookInterpreter', 'Shutdown Kernel'), + positronActionBarOptions: { + controlType: 'button', + displayTitle: false + }, + icon: Codicon.positronPowerButtonThin, + f1: true, + category, + precondition: ActiveNotebookHasRunningRuntime, + menu: [ + // Positron notebooks + { + id: MenuId.PositronNotebookKernelSubmenu, + order: 20, + } + ] + }); + } + + override async runWithContext(accessor: ServicesAccessor, context?: IRuntimeNotebookKernelActionContext): Promise { + const session = context?.runtimeSession; if (!session) { throw new Error('No session found for active notebook. This command should only be available when a session is running.'); } - // Restart the session with a progress bar. + const progressService = accessor.get(IProgressService); + const notificationService = accessor.get(INotificationService); + const runtimeSessionService = accessor.get(IRuntimeSessionService); + + // Shutdown the session with a progress bar. try { await progressService.withProgress({ location: ProgressLocation.Notification, - title: localize("positron.notebook.restart.restarting", "Restarting {0} interpreter for '{1}'", - session.runtimeMetadata.runtimeName, notebookUri.fsPath), - }, () => runtimeSessionService.restartSession(session.metadata.sessionId, - context ? - 'User clicked restart button in the notebook editor toolbar' : - `Restart notebook kernel command ${RuntimeNotebookKernelRestartAction.ID} executed`)); + title: localize("positron.notebook.shutdown.shuttingDown", "Shutting down {0} interpreter for '{1}'", + session.runtimeMetadata.runtimeName, session.metadata.notebookUri.fsPath), + }, () => runtimeSessionService.shutdownNotebookSession(session.metadata.notebookUri, RuntimeExitReason.Shutdown, context.source.debugMessage)); } catch (error) { notificationService.error( - localize("positron.notebook.restart.failed", "Restarting {0} interpreter for '{1}' failed. Reason: {2}", - session.runtimeMetadata.runtimeName, notebookUri.fsPath, error.message)); + localize("positron.notebook.shutdown.failed", "Shutting down {0} interpreter for '{1}' failed. Reason: {2}", + session.runtimeMetadata.runtimeName, session.metadata.notebookUri.fsPath, error.message)); } } } export function registerRuntimeNotebookKernelActions(): void { registerAction2(RuntimeNotebookKernelRestartAction); + registerAction2(RuntimeNotebookKernelShutdownAction); } diff --git a/src/vs/workbench/contrib/runtimeNotebookKernel/common/activeRuntimeNotebookContextManager.ts b/src/vs/workbench/contrib/runtimeNotebookKernel/common/activeRuntimeNotebookContextManager.ts index c98f5089e1b2..ad31d8adf621 100644 --- a/src/vs/workbench/contrib/runtimeNotebookKernel/common/activeRuntimeNotebookContextManager.ts +++ b/src/vs/workbench/contrib/runtimeNotebookKernel/common/activeRuntimeNotebookContextManager.ts @@ -8,10 +8,12 @@ import { isEqual } from '../../../../base/common/resources.js'; import { URI } from '../../../../base/common/uri.js'; import { localize } from '../../../../nls.js'; import { IContextKey, IContextKeyService, RawContextKey } from '../../../../platform/contextkey/common/contextkey.js'; +import { EditorInput } from '../../../common/editor/editorInput.js'; import { IEditorService } from '../../../services/editor/common/editorService.js'; import { ILanguageRuntimeInfo, LanguageRuntimeSessionMode, RuntimeState } from '../../../services/languageRuntime/common/languageRuntimeService.js'; import { ILanguageRuntimeSession, IRuntimeSessionService } from '../../../services/runtimeSession/common/runtimeSessionService.js'; -import { isNotebookEditorInput } from '../../notebook/common/notebookEditorInput.js'; +import { isNotebookEditorInput as isVSCodeNotebookEditorInput } from '../../notebook/common/notebookEditorInput.js'; +import { POSITRON_NOTEBOOK_EDITOR_INPUT_ID } from '../../positronNotebook/common/positronNotebookCommon.js'; /** Whether the active notebook has a running runtime. */ export const ActiveNotebookHasRunningRuntime = new RawContextKey( @@ -163,3 +165,10 @@ export class ActiveRuntimeNotebookContextManager extends Disposable { ); } } + +export function isNotebookEditorInput(editor: EditorInput | undefined): editor is EditorInput & { resource: URI } { + return editor !== undefined && ( + isVSCodeNotebookEditorInput(editor) || + (editor.typeId === POSITRON_NOTEBOOK_EDITOR_INPUT_ID && editor.resource !== undefined) + ); +} diff --git a/src/vs/workbench/contrib/runtimeNotebookKernel/tests/browser/activeRuntimeNotebookContextManager.test.ts b/src/vs/workbench/contrib/runtimeNotebookKernel/tests/browser/activeRuntimeNotebookContextManager.test.ts index 82182b338d2c..b90a7528ae14 100644 --- a/src/vs/workbench/contrib/runtimeNotebookKernel/tests/browser/activeRuntimeNotebookContextManager.test.ts +++ b/src/vs/workbench/contrib/runtimeNotebookKernel/tests/browser/activeRuntimeNotebookContextManager.test.ts @@ -21,6 +21,7 @@ import { TestEditorInput, TestEditorService } from '../../../../test/browser/wor import { NotebookEditorInput } from '../../../notebook/common/notebookEditorInput.js'; import { ActiveRuntimeNotebookContextManager, DebuggerRuntimeSupportedFeature } from '../../common/activeRuntimeNotebookContextManager.js'; import { isEqual } from '../../../../../base/common/resources.js'; +import { isNotebookLanguageRuntimeSession } from '../../../../services/runtimeSession/common/runtimeSession.js'; /** A TestEditorService that fires the onDidActiveEditorChange event when changing the activeEditor. */ class TestEditorService2 extends TestEditorService { @@ -100,7 +101,7 @@ class MockRuntimeSessionService extends Disposable implements Partial isEqual(session.metadata.notebookUri, notebookUri)); + return this.activeSessions.filter(isNotebookLanguageRuntimeSession).find(session => isEqual(session.metadata.notebookUri, notebookUri)); } } diff --git a/src/vs/workbench/services/positronHistory/test/common/executionHistoryService.test.ts b/src/vs/workbench/services/positronHistory/test/common/executionHistoryService.test.ts index 814f2ea15be1..2ca3fdf2cdce 100644 --- a/src/vs/workbench/services/positronHistory/test/common/executionHistoryService.test.ts +++ b/src/vs/workbench/services/positronHistory/test/common/executionHistoryService.test.ts @@ -9,7 +9,7 @@ import { Disposable, IDisposable } from '../../../../../base/common/lifecycle.js import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { ILogService, NullLogService } from '../../../../../platform/log/common/log.js'; import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; -import { ILanguageRuntimeSession, IRuntimeSessionService, RuntimeStartMode, ILanguageRuntimeSessionStateEvent, ILanguageRuntimeGlobalEvent, IRuntimeSessionMetadata, IRuntimeSessionWillStartEvent, INotebookSessionUriChangedEvent } from '../../../../services/runtimeSession/common/runtimeSessionService.js'; +import { ILanguageRuntimeSession, IRuntimeSessionService, RuntimeStartMode, ILanguageRuntimeSessionStateEvent, ILanguageRuntimeGlobalEvent, IRuntimeSessionMetadata, IRuntimeSessionWillStartEvent, INotebookSessionUriChangedEvent, INotebookLanguageRuntimeSession } from '../../../../services/runtimeSession/common/runtimeSessionService.js'; import { IExecutionHistoryService, ExecutionEntryType } from '../../common/executionHistoryService.js'; import { IRuntimeAutoStartEvent, IRuntimeStartupService, ISessionRestoreFailedEvent, SerializedSessionMetadata } from '../../../../services/runtimeStartup/common/runtimeStartupService.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; @@ -156,7 +156,7 @@ class TestRuntimeSessionService implements IRuntimeSessionService { throw new Error('Method not implemented.'); } - getNotebookSessionForNotebookUri(_notebookUri: any): ILanguageRuntimeSession | undefined { + getNotebookSessionForNotebookUri(_notebookUri: any): INotebookLanguageRuntimeSession | undefined { throw new Error('Method not implemented.'); } diff --git a/src/vs/workbench/services/runtimeSession/common/runtimeSession.ts b/src/vs/workbench/services/runtimeSession/common/runtimeSession.ts index 567a603625e8..5997bba923a9 100644 --- a/src/vs/workbench/services/runtimeSession/common/runtimeSession.ts +++ b/src/vs/workbench/services/runtimeSession/common/runtimeSession.ts @@ -12,7 +12,7 @@ import { InstantiationType, registerSingleton } from '../../../../platform/insta import { ILogService } from '../../../../platform/log/common/log.js'; import { IOpener, IOpenerService, OpenExternalOptions, OpenInternalOptions } from '../../../../platform/opener/common/opener.js'; import { ILanguageRuntimeMetadata, ILanguageRuntimeService, LanguageRuntimeSessionLocation, LanguageRuntimeSessionMode, LanguageRuntimeStartupBehavior, RuntimeExitReason, RuntimeState, LanguageStartupBehavior, formatLanguageRuntimeMetadata, formatLanguageRuntimeSession } from '../../languageRuntime/common/languageRuntimeService.js'; -import { ILanguageRuntimeGlobalEvent, ILanguageRuntimeSession, ILanguageRuntimeSessionManager, ILanguageRuntimeSessionStateEvent, INotebookSessionUriChangedEvent, IRuntimeSessionMetadata, IRuntimeSessionService, IRuntimeSessionWillStartEvent, RuntimeStartMode } from './runtimeSessionService.js'; +import { ILanguageRuntimeGlobalEvent, INotebookLanguageRuntimeSession, ILanguageRuntimeSession, ILanguageRuntimeSessionManager, ILanguageRuntimeSessionStateEvent, INotebookSessionUriChangedEvent, IRuntimeSessionMetadata, IRuntimeSessionService, IRuntimeSessionWillStartEvent, RuntimeStartMode } from './runtimeSessionService.js'; import { IWorkspaceTrustManagementService } from '../../../../platform/workspace/common/workspaceTrust.js'; import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; import { IModalDialogPromptInstance, IPositronModalDialogsService } from '../../positronModalDialogs/common/positronModalDialogs.js'; @@ -106,7 +106,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession // A map of the currently active notebook sessions. This is keyed by the notebook URI // owning the session. - private readonly _notebookSessionsByNotebookUri = new ResourceMap(); + private readonly _notebookSessionsByNotebookUri = new ResourceMap(); // An map of sessions that have been disconnected from the extension host, // from sessionId to session. We keep these around so we can reconnect them when @@ -373,7 +373,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession * @returns The notebook session with the given notebook URI, or undefined if * no notebook session with the given notebook URI exists. */ - getNotebookSessionForNotebookUri(notebookUri: URI): ILanguageRuntimeSession | undefined { + getNotebookSessionForNotebookUri(notebookUri: URI): INotebookLanguageRuntimeSession | undefined { const session = this._notebookSessionsByNotebookUri.get(notebookUri); this._logService.info(`Lookup notebook session for notebook URI ${notebookUri.toString()}: ${session ? session.metadata.sessionId : 'not found'}`); return session; @@ -689,7 +689,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession const createConsole = sessionMode === LanguageRuntimeSessionMode.Console || ( sessionMode === LanguageRuntimeSessionMode.Notebook && - this._configurationService.getValue("console.showNotebookConsoles")); + this._configurationService.getValue('console.showNotebookConsoles')); // Start the runtime. this._logService.info( @@ -1731,7 +1731,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession // Append the new session to the list of existing sessions if it hasn't been added this.addSessionToConsoleSessionMap(session); } else if (session.metadata.sessionMode === LanguageRuntimeSessionMode.Notebook) { - if (session.metadata.notebookUri) { + if (isNotebookLanguageRuntimeSession(session)) { this._logService.info(`Notebook session for ${session.metadata.notebookUri} started: ${session.metadata.sessionId}`); this._notebookSessionsByNotebookUri.set(session.metadata.notebookUri, session); } else { @@ -1837,8 +1837,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession // Restore the session in the case of a restart. if (session.metadata.sessionMode === LanguageRuntimeSessionMode.Console) { this.addSessionToConsoleSessionMap(session); - } else if (session.metadata.sessionMode === LanguageRuntimeSessionMode.Notebook && - session.metadata.notebookUri && + } else if (isNotebookLanguageRuntimeSession(session) && !this._notebookSessionsByNotebookUri.has(session.metadata.notebookUri)) { this._notebookSessionsByNotebookUri.set(session.metadata.notebookUri, session); } @@ -2465,6 +2464,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession localize('positron.notebook.workingDirectoryChanged.title', 'Update working directory?'), localize( 'positron.notebook.workingDirectoryChanged', + // eslint-disable-next-line local/code-no-unexternalized-strings 'This notebook was moved to a new location but your session is still running from the original directory.' + '

Saved at: {0}' + '
Running from: {1}' @@ -2532,3 +2532,12 @@ function awaitStateChange( } }); } + +/** + * Checks whether a session is for a notebook. + * @param session The session to check + */ +export function isNotebookLanguageRuntimeSession(session: ILanguageRuntimeSession): session is INotebookLanguageRuntimeSession { + return session.metadata.notebookUri !== undefined && + session.metadata.sessionMode === LanguageRuntimeSessionMode.Notebook; +} diff --git a/src/vs/workbench/services/runtimeSession/common/runtimeSessionService.ts b/src/vs/workbench/services/runtimeSession/common/runtimeSessionService.ts index eeb3576ac439..b4d1285566a3 100644 --- a/src/vs/workbench/services/runtimeSession/common/runtimeSessionService.ts +++ b/src/vs/workbench/services/runtimeSession/common/runtimeSessionService.ts @@ -241,6 +241,14 @@ export interface ILanguageRuntimeSession extends IDisposable { updateSessionName(sessionName: string): void; } +export interface INotebookRuntimeSessionMetadata extends IRuntimeSessionMetadata { + notebookUri: URI; +} + +export interface INotebookLanguageRuntimeSession extends ILanguageRuntimeSession { + metadata: INotebookRuntimeSessionMetadata; +} + /** * A manager for runtime sessions. */ @@ -385,7 +393,7 @@ export interface IRuntimeSessionService { * Gets a specific notebook session by notebook URI. Currently, only one * notebook session can exist per notebook URI. */ - getNotebookSessionForNotebookUri(notebookUri: URI): ILanguageRuntimeSession | undefined; + getNotebookSessionForNotebookUri(notebookUri: URI): INotebookLanguageRuntimeSession | undefined; /** * List all active runtime sessions. diff --git a/test/e2e/infra/workbench.ts b/test/e2e/infra/workbench.ts index c708e34de114..4e752a58069b 100644 --- a/test/e2e/infra/workbench.ts +++ b/test/e2e/infra/workbench.ts @@ -119,7 +119,7 @@ export class Workbench { this.sessions = new Sessions(code, this.quickaccess, this.quickInput, this.console, this.contextMenu); this.notebooks = new Notebooks(code, this.quickInput, this.quickaccess, this.hotKeys); this.notebooksVscode = new VsCodeNotebooks(code, this.quickInput, this.quickaccess, this.hotKeys); - this.notebooksPositron = new PositronNotebooks(code, this.quickInput, this.quickaccess, this.hotKeys, this.clipboard); + this.notebooksPositron = new PositronNotebooks(code, this.quickInput, this.quickaccess, this.hotKeys, this.clipboard, this.contextMenu); this.welcome = new Welcome(code); this.terminal = new Terminal(code, this.quickaccess, this.clipboard); this.viewer = new Viewer(code); diff --git a/test/e2e/pages/notebooksPositron.ts b/test/e2e/pages/notebooksPositron.ts index 77690feff6e2..81c0dfccf279 100644 --- a/test/e2e/pages/notebooksPositron.ts +++ b/test/e2e/pages/notebooksPositron.ts @@ -10,6 +10,7 @@ import { QuickAccess } from './quickaccess'; import { Clipboard } from './clipboard.js'; import test, { expect, Locator } from '@playwright/test'; import { HotKeys } from './hotKeys.js'; +import { ContextMenu } from './dialog-contextMenu.js'; const DEFAULT_TIMEOUT = 10000; @@ -35,7 +36,7 @@ export class PositronNotebooks extends Notebooks { moreActionsButtonAtIndex = (index: number) => this.cell.nth(index).getByRole('button', { name: /more actions/i }); moreActionsOption = (option: string) => this.code.driver.page.locator('button.custom-context-menu-item', { hasText: option }); - constructor(code: Code, quickinput: QuickInput, quickaccess: QuickAccess, hotKeys: HotKeys, private clipboard: Clipboard) { + constructor(code: Code, quickinput: QuickInput, quickaccess: QuickAccess, hotKeys: HotKeys, private clipboard: Clipboard, private contextMenu: ContextMenu) { super(code, quickinput, quickaccess, hotKeys); } @@ -378,13 +379,17 @@ export class PositronNotebooks extends Notebooks { this.code.logger.log(`Clicking kernel status badge to select: ${desiredKernel}`); await expect(async () => { // we shouldn't need to retry this, but the input closes immediately sometimes - await this.kernelStatusBadge.click(); + await this.contextMenu.triggerAndClick({ + menuTrigger: this.kernelStatusBadge, + menuItemLabel: `Change Kernel...` + }); + // this is a short wait because for some reason, 1st click always gets auto-closed in playwright :shrug: await this.quickinput.waitForQuickInputOpened({ timeout: 1000 }); - }).toPass({ timeout: 10000 }); - // Select the desired kernel - await this.quickinput.selectQuickInputElementContaining(desiredKernel); - await this.quickinput.waitForQuickInputClosed(); + // Select the desired kernel + await this.quickinput.selectQuickInputElementContaining(desiredKernel); + await this.quickinput.waitForQuickInputClosed(); + }).toPass({ timeout: 10000 }); this.code.logger.log(`Selected kernel: ${desiredKernel}`); } catch (e) { @@ -393,7 +398,7 @@ export class PositronNotebooks extends Notebooks { } // Wait for the kernel status to show "Connected" - await expect(this.kernelStatusBadge).toContainText('Connected', { timeout: 30000 }); + await expect(this.kernelStatusBadge).toContainText(desiredKernel, { timeout: 30000 }); this.code.logger.log('Kernel is connected and ready'); }); } diff --git a/test/e2e/pages/sessions.ts b/test/e2e/pages/sessions.ts index b5f4d530f4c2..5b781d1f1d71 100644 --- a/test/e2e/pages/sessions.ts +++ b/test/e2e/pages/sessions.ts @@ -15,7 +15,7 @@ const getAlternateR = () => process.env.POSITRON_R_ALT_VER_SEL; const getHiddenPython = () => process.env.POSITRON_HIDDEN_PY; const getHiddenR = () => process.env.POSITRON_HIDDEN_R; -const ACTIVE_STATUS_ICON = '.codicon-positron-status-active'; +const ACTIVE_STATUS_ICON = '.codicon-positron-runtime-status-active'; /** * Class to manage console sessions @@ -34,8 +34,8 @@ export class Sessions { // Session status indicators private activeStatus = (session: Locator) => session.locator(ACTIVE_STATUS_ICON); - private idleStatus = (session: Locator) => session.locator('.codicon-positron-status-idle'); - private disconnectedStatus = (session: Locator) => session.locator('.codicon-positron-status-disconnected'); + private idleStatus = (session: Locator) => session.locator('.codicon-positron-runtime-status-idle'); + private disconnectedStatus = (session: Locator) => session.locator('.codicon-positron-runtime-status-disconnected'); private activeStatusIcon = this.page.locator(ACTIVE_STATUS_ICON); // Session Metadata @@ -660,7 +660,7 @@ export class Sessions { const activeSessions = ( await Promise.all( allSessionTabs.map(async session => { - const isDisconnected = await session.locator('.codicon-positron-status-disconnected').isVisible(); + const isDisconnected = await session.locator('.codicon-positron-runtime-status-disconnected').isVisible(); if (isDisconnected) { return null; } // Extract session ID from data-testid attribute @@ -796,7 +796,7 @@ export class Sessions { if (sessionCount > 1) { // get status from icon in tab list view const sessionTab = this.getSessionTab(sessionIdOrName); - const statusClass = `.codicon-positron-status-${expectedStatus}`; + const statusClass = `.codicon-positron-runtime-status-${expectedStatus}`; await expect(sessionTab).toBeVisible(); await expect(sessionTab.locator(statusClass)).toBeVisible({ timeout }); diff --git a/test/e2e/tests/notebook/cell-execution-info.test.ts b/test/e2e/tests/notebook/cell-execution-info.test.ts index 78dc04bb2675..2c56704381e7 100644 --- a/test/e2e/tests/notebook/cell-execution-info.test.ts +++ b/test/e2e/tests/notebook/cell-execution-info.test.ts @@ -26,7 +26,7 @@ test.describe('Positron Notebooks: Cell Execution Tooltip', { await hotKeys.closeAllEditors(); }); - test('Cell Execution Tooltip - Basic Functionality', async function ({ app }) { + test.skip('Cell Execution Tooltip - Basic Functionality', async function ({ app }) { const { notebooks, notebooksPositron } = app.workbench; await test.step('Test Setup: Create notebook and select kernel', async () => {