From 6779fd96bfbfa297e643bf0c76329a4a63bf341d Mon Sep 17 00:00:00 2001 From: Andrea Mah <31675041+andreamah@users.noreply.github.com> Date: Fri, 15 Jul 2022 09:52:17 -0700 Subject: [PATCH] Add "Go to Last Failed Cell" Button (#154443) * Add go to last failed cell function --- .../browser/controller/executeActions.ts | 52 ++++++++++++++++++- .../browser/media/notebookToolbar.css | 4 ++ .../notebookExecutionStateServiceImpl.ts | 36 +++++++++++-- .../notebookEditorWidgetContextKeys.ts | 13 ++++- .../notebook/common/notebookContextKeys.ts | 1 + .../common/notebookExecutionStateService.ts | 6 +++ .../test/browser/testNotebookEditor.ts | 8 ++- 7 files changed, 111 insertions(+), 9 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/controller/executeActions.ts b/src/vs/workbench/contrib/notebook/browser/controller/executeActions.ts index 3e54598b92b52..7b190d89c14e2 100644 --- a/src/vs/workbench/contrib/notebook/browser/controller/executeActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/controller/executeActions.ts @@ -15,7 +15,7 @@ import { ThemeIcon } from 'vs/platform/theme/common/themeService'; import { EditorsOrder } from 'vs/workbench/common/editor'; import { insertCell } from 'vs/workbench/contrib/notebook/browser/controller/cellOperations'; import { cellExecutionArgs, CellToolbarOrder, CELL_TITLE_CELL_GROUP_ID, executeNotebookCondition, getContextFromActiveEditor, getContextFromUri, INotebookActionContext, INotebookCellActionContext, INotebookCellToolbarActionContext, INotebookCommandContext, NotebookAction, NotebookCellAction, NotebookMultiCellAction, NOTEBOOK_EDITOR_WIDGET_ACTION_WEIGHT, parseMultiCellExecutionArgs } from 'vs/workbench/contrib/notebook/browser/controller/coreActions'; -import { NOTEBOOK_CELL_EXECUTING, NOTEBOOK_CELL_EXECUTION_STATE, NOTEBOOK_CELL_LIST_FOCUSED, NOTEBOOK_CELL_TYPE, NOTEBOOK_HAS_RUNNING_CELL, NOTEBOOK_INTERRUPTIBLE_KERNEL, NOTEBOOK_IS_ACTIVE_EDITOR, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_KERNEL_SOURCE_COUNT, NOTEBOOK_MISSING_KERNEL_EXTENSION } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; +import { NOTEBOOK_CELL_EXECUTING, NOTEBOOK_CELL_EXECUTION_STATE, NOTEBOOK_CELL_LIST_FOCUSED, NOTEBOOK_CELL_TYPE, NOTEBOOK_HAS_RUNNING_CELL, NOTEBOOK_INTERRUPTIBLE_KERNEL, NOTEBOOK_IS_ACTIVE_EDITOR, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_KERNEL_SOURCE_COUNT, NOTEBOOK_LAST_CELL_FAILED, NOTEBOOK_MISSING_KERNEL_EXTENSION } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; import { CellEditState, CellFocusMode, EXECUTE_CELL_COMMAND_ID } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import * as icons from 'vs/workbench/contrib/notebook/browser/notebookIcons'; import { CellKind, NotebookSetting } from 'vs/workbench/contrib/notebook/common/notebookCommon'; @@ -35,6 +35,7 @@ const EXECUTE_CELL_AND_BELOW = 'notebook.cell.executeCellAndBelow'; const EXECUTE_CELLS_ABOVE = 'notebook.cell.executeCellsAbove'; const RENDER_ALL_MARKDOWN_CELLS = 'notebook.renderAllMarkdownCells'; const REVEAL_RUNNING_CELL = 'notebook.revealRunningCell'; +const REVEAL_LAST_FAILED_CELL = 'notebook.revealLastFailedCell'; // If this changes, update getCodeCellExecutionContextKeyService to match export const executeCondition = ContextKeyExpr.and( @@ -594,3 +595,52 @@ registerAction2(class RevealRunningCellAction extends NotebookAction { } } }); + +registerAction2(class RevealLastFailedCellAction extends NotebookAction { + constructor() { + super({ + id: REVEAL_LAST_FAILED_CELL, + title: localize('revealLastFailedCell', "Go to Most Recently Failed Cell"), + tooltip: localize('revealLastFailedCell', "Go to Most Recently Failed Cell"), + shortTitle: localize('revealLastFailedCellShort', "Go To"), + precondition: NOTEBOOK_LAST_CELL_FAILED, + menu: [ + { + id: MenuId.EditorTitle, + when: ContextKeyExpr.and( + NOTEBOOK_IS_ACTIVE_EDITOR, + NOTEBOOK_LAST_CELL_FAILED, + NOTEBOOK_HAS_RUNNING_CELL.toNegated(), + ContextKeyExpr.notEquals('config.notebook.globalToolbar', true) + ), + group: 'navigation', + order: 0 + }, + { + id: MenuId.NotebookToolbar, + when: ContextKeyExpr.and( + NOTEBOOK_IS_ACTIVE_EDITOR, + NOTEBOOK_LAST_CELL_FAILED, + NOTEBOOK_HAS_RUNNING_CELL.toNegated(), + ContextKeyExpr.equals('config.notebook.globalToolbar', true) + ), + group: 'navigation/execute', + order: 0 + }, + ], + icon: icons.errorStateIcon, + }); + } + + async runWithContext(accessor: ServicesAccessor, context: INotebookActionContext): Promise { + const notebookExecutionStateService = accessor.get(INotebookExecutionStateService); + const notebook = context.notebookEditor.textModel.uri; + const lastFailedCellHandle = notebookExecutionStateService.getLastFailedCellForNotebook(notebook); + if (lastFailedCellHandle !== undefined) { + const lastFailedCell = context.notebookEditor.getCellByHandle(lastFailedCellHandle); + if (lastFailedCell) { + context.notebookEditor.focusNotebookCell(lastFailedCell, 'container'); + } + } + } +}); diff --git a/src/vs/workbench/contrib/notebook/browser/media/notebookToolbar.css b/src/vs/workbench/contrib/notebook/browser/media/notebookToolbar.css index 4b61bafdfd84e..f7ddb6665a70a 100644 --- a/src/vs/workbench/contrib/notebook/browser/media/notebookToolbar.css +++ b/src/vs/workbench/contrib/notebook/browser/media/notebookToolbar.css @@ -80,3 +80,7 @@ .monaco-workbench .notebookOverlay .notebook-toolbar-container .monaco-action-bar:not(.vertical) .action-item.active { background-color: unset; } + +.monaco-workbench .notebookOverlay .notebook-toolbar-container .monaco-action-bar .action-item .codicon-notebook-state-error { + color: var(--notebook-cell-status-icon-error); +} diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts index e32dbce7ba8f8..0cf3e56c598ef 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts @@ -13,7 +13,7 @@ import { ILogService } from 'vs/platform/log/common/log'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { CellEditType, CellUri, ICellEditOperation, NotebookCellExecutionState, NotebookCellInternalMetadata, NotebookTextModelWillAddRemoveEvent } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { CellExecutionUpdateType, INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; -import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionStateChangedEvent, ICellExecutionStateUpdate, INotebookCellExecution, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionStateChangedEvent, ICellExecutionStateUpdate, INotebookCellExecution, INotebookExecutionStateService, INotebookFailStateChangedEvent } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; export class NotebookExecutionStateService extends Disposable implements INotebookExecutionStateService { @@ -22,10 +22,14 @@ export class NotebookExecutionStateService extends Disposable implements INotebo private readonly _executions = new ResourceMap>(); private readonly _notebookListeners = new ResourceMap(); private readonly _cellListeners = new ResourceMap(); + private readonly _lastFailedCells = new ResourceMap(); private readonly _onDidChangeCellExecution = this._register(new Emitter()); onDidChangeCellExecution = this._onDidChangeCellExecution.event; + private readonly _onDidChangeLastRunFailState = this._register(new Emitter()); + onDidChangeLastRunFailState = this._onDidChangeLastRunFailState.event; + constructor( @IInstantiationService private readonly _instantiationService: IInstantiationService, @ILogService private readonly _logService: ILogService, @@ -34,6 +38,10 @@ export class NotebookExecutionStateService extends Disposable implements INotebo super(); } + getLastFailedCellForNotebook(notebook: URI): number | undefined { + return this._lastFailedCells.get(notebook); + } + forceCancelNotebookExecutions(notebookUri: URI): void { const notebookExecutions = this._executions.get(notebookUri); if (!notebookExecutions) { @@ -68,7 +76,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo this._onDidChangeCellExecution.fire(new NotebookExecutionEvent(notebookUri, cellHandle, exe)); } - private _onCellExecutionDidComplete(notebookUri: URI, cellHandle: number, exe: CellExecution): void { + private _onCellExecutionDidComplete(notebookUri: URI, cellHandle: number, exe: CellExecution, lastRunSuccess?: boolean): void { const notebookExecutions = this._executions.get(notebookUri); if (!notebookExecutions) { this._logService.debug(`NotebookExecutionStateService#_onCellExecutionDidComplete - unknown notebook ${notebookUri.toString()}`); @@ -86,6 +94,14 @@ export class NotebookExecutionStateService extends Disposable implements INotebo this._notebookListeners.delete(notebookUri); } + if (lastRunSuccess !== undefined) { + if (lastRunSuccess) { + this._clearLastFailedCell(notebookUri); + } else { + this._setLastFailedCell(notebookUri, cellHandle); + } + } + this._onDidChangeCellExecution.fire(new NotebookExecutionEvent(notebookUri, cellHandle)); } @@ -119,12 +135,22 @@ export class NotebookExecutionStateService extends Disposable implements INotebo const exe: CellExecution = this._instantiationService.createInstance(CellExecution, cellHandle, notebook); const disposable = combinedDisposable( exe.onDidUpdate(() => this._onCellExecutionDidChange(notebookUri, cellHandle, exe)), - exe.onDidComplete(() => this._onCellExecutionDidComplete(notebookUri, cellHandle, exe))); + exe.onDidComplete(lastRunSuccess => this._onCellExecutionDidComplete(notebookUri, cellHandle, exe, lastRunSuccess))); this._cellListeners.set(CellUri.generate(notebookUri, cellHandle), disposable); return exe; } + private _setLastFailedCell(notebook: URI, cellHandle: number) { + this._lastFailedCells.set(notebook, cellHandle); + this._onDidChangeLastRunFailState.fire({ failed: true, notebook }); + } + + private _clearLastFailedCell(notebook: URI) { + this._lastFailedCells.delete(notebook); + this._onDidChangeLastRunFailState.fire({ failed: false, notebook: notebook }); + } + override dispose(): void { super.dispose(); this._executions.forEach(executionMap => { @@ -250,7 +276,7 @@ class CellExecution extends Disposable implements INotebookCellExecution { private readonly _onDidUpdate = this._register(new Emitter()); readonly onDidUpdate = this._onDidUpdate.event; - private readonly _onDidComplete = this._register(new Emitter()); + private readonly _onDidComplete = this._register(new Emitter()); readonly onDidComplete = this._onDidComplete.event; private _state: NotebookCellExecutionState = NotebookCellExecutionState.Unconfirmed; @@ -350,7 +376,7 @@ class CellExecution extends Disposable implements INotebookCellExecution { this._applyExecutionEdits([edit]); } - this._onDidComplete.fire(); + this._onDidComplete.fire(completionData.lastRunSuccess); } private _applyExecutionEdits(edits: ICellEditOperation[]): void { diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts index 3702ac26261f5..27384f830d49d 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts @@ -6,8 +6,8 @@ import { DisposableStore, dispose, IDisposable } from 'vs/base/common/lifecycle'; import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { ICellViewModel, INotebookEditorDelegate, KERNEL_EXTENSIONS } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { NOTEBOOK_CELL_TOOLBAR_LOCATION, NOTEBOOK_HAS_OUTPUTS, NOTEBOOK_HAS_RUNNING_CELL, NOTEBOOK_INTERRUPTIBLE_KERNEL, NOTEBOOK_KERNEL, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_KERNEL_SELECTED, NOTEBOOK_KERNEL_SOURCE_COUNT, NOTEBOOK_MISSING_KERNEL_EXTENSION, NOTEBOOK_USE_CONSOLIDATED_OUTPUT_BUTTON, NOTEBOOK_VIEW_TYPE } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; -import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { NOTEBOOK_CELL_TOOLBAR_LOCATION, NOTEBOOK_HAS_OUTPUTS, NOTEBOOK_HAS_RUNNING_CELL, NOTEBOOK_INTERRUPTIBLE_KERNEL, NOTEBOOK_KERNEL, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_KERNEL_SELECTED, NOTEBOOK_KERNEL_SOURCE_COUNT, NOTEBOOK_LAST_CELL_FAILED, NOTEBOOK_MISSING_KERNEL_EXTENSION, NOTEBOOK_USE_CONSOLIDATED_OUTPUT_BUTTON, NOTEBOOK_VIEW_TYPE } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; +import { INotebookExecutionStateService, INotebookFailStateChangedEvent } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; @@ -24,6 +24,7 @@ export class NotebookEditorContextKeys { private readonly _viewType!: IContextKey; private readonly _missingKernelExtension: IContextKey; private readonly _cellToolbarLocation: IContextKey<'left' | 'right' | 'hidden'>; + private readonly _lastCellFailed: IContextKey; private readonly _disposables = new DisposableStore(); private readonly _viewModelDisposables = new DisposableStore(); @@ -47,6 +48,7 @@ export class NotebookEditorContextKeys { this._missingKernelExtension = NOTEBOOK_MISSING_KERNEL_EXTENSION.bindTo(contextKeyService); this._notebookKernelSourceCount = NOTEBOOK_KERNEL_SOURCE_COUNT.bindTo(contextKeyService); this._cellToolbarLocation = NOTEBOOK_CELL_TOOLBAR_LOCATION.bindTo(contextKeyService); + this._lastCellFailed = NOTEBOOK_LAST_CELL_FAILED.bindTo(contextKeyService); this._handleDidChangeModel(); this._updateForNotebookOptions(); @@ -58,6 +60,7 @@ export class NotebookEditorContextKeys { this._disposables.add(_editor.notebookOptions.onDidChangeOptions(this._updateForNotebookOptions, this)); this._disposables.add(_extensionService.onDidChangeExtensions(this._updateForInstalledExtension, this)); this._disposables.add(_notebookExecutionStateService.onDidChangeCellExecution(this._updateForCellExecution, this)); + this._disposables.add(_notebookExecutionStateService.onDidChangeLastRunFailState(this._updateForLastRunFailState, this)); } dispose(): void { @@ -132,6 +135,12 @@ export class NotebookEditorContextKeys { } } + private _updateForLastRunFailState(e: INotebookFailStateChangedEvent): void { + if (e.notebook === this._editor.textModel?.uri) { + this._lastCellFailed.set(e.failed); + } + } + private async _updateForInstalledExtension(): Promise { if (!this._editor.hasModel()) { return; diff --git a/src/vs/workbench/contrib/notebook/common/notebookContextKeys.ts b/src/vs/workbench/contrib/notebook/common/notebookContextKeys.ts index e629ed034d26b..fd9692eba0f4f 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookContextKeys.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookContextKeys.ts @@ -24,6 +24,7 @@ export const NOTEBOOK_USE_CONSOLIDATED_OUTPUT_BUTTON = new RawContextKey('notebookBreakpointMargin', false); export const NOTEBOOK_CELL_TOOLBAR_LOCATION = new RawContextKey<'left' | 'right' | 'hidden'>('notebookCellToolbarLocation', 'left'); export const NOTEBOOK_CURSOR_NAVIGATION_MODE = new RawContextKey('notebookCursorNavigationMode', false); +export const NOTEBOOK_LAST_CELL_FAILED = new RawContextKey('notebookLastCellFailed', false); // Cell keys export const NOTEBOOK_VIEW_TYPE = new RawContextKey('notebookType', undefined); diff --git a/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts b/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts index f6317245bb18e..d1fbe75907f49 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts @@ -39,6 +39,10 @@ export interface ICellExecutionStateChangedEvent { affectsCell(cell: URI): boolean; affectsNotebook(notebook: URI): boolean; } +export interface INotebookFailStateChangedEvent { + failed: boolean; + notebook: URI; +} export const INotebookExecutionStateService = createDecorator('INotebookExecutionStateService'); @@ -46,11 +50,13 @@ export interface INotebookExecutionStateService { _serviceBrand: undefined; onDidChangeCellExecution: Event; + onDidChangeLastRunFailState: Event; forceCancelNotebookExecutions(notebookUri: URI): void; getCellExecutionStatesForNotebook(notebook: URI): INotebookCellExecution[]; getCellExecution(cellUri: URI): INotebookCellExecution | undefined; createCellExecution(notebook: URI, cellHandle: number): INotebookCellExecution; + getLastFailedCellForNotebook(notebook: URI): number | undefined; } export interface INotebookCellExecution { diff --git a/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts b/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts index 8570bf9146a1a..6b16b20461f1c 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts @@ -44,7 +44,7 @@ import { ViewContext } from 'vs/workbench/contrib/notebook/browser/viewModel/vie import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { CellKind, CellUri, INotebookDiffEditorModel, INotebookEditorModel, INotebookSearchOptions, IOutputDto, IResolvedNotebookEditorModel, NotebookCellExecutionState, NotebookCellMetadata, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionStateChangedEvent, INotebookCellExecution, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionStateChangedEvent, INotebookCellExecution, INotebookExecutionStateService, INotebookFailStateChangedEvent } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { NotebookOptions } from 'vs/workbench/contrib/notebook/common/notebookOptions'; import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange'; import { TextModelResolverService } from 'vs/workbench/services/textmodelResolver/common/textModelResolverService'; @@ -405,11 +405,17 @@ class TestCellExecution implements INotebookCellExecution { } class TestNotebookExecutionStateService implements INotebookExecutionStateService { + + getLastFailedCellForNotebook(notebook: URI): number | undefined { + return; + } + _serviceBrand: undefined; private _executions = new ResourceMap(); onDidChangeCellExecution = new Emitter().event; + onDidChangeLastRunFailState = new Emitter().event; forceCancelNotebookExecutions(notebookUri: URI): void { }