Skip to content

Commit a832f99

Browse files
authored
add a toolbar item to open cell diagnostic actions (microsoft#208358)
* add a toolbar item to open cell diagnostic actions * fix updated diagnostic handling * prioritize to be next to cell result * dispose status bar items, better handling of non-code cell
1 parent 9d6c275 commit a832f99

File tree

7 files changed

+86
-20
lines changed

7 files changed

+86
-20
lines changed

src/vs/workbench/contrib/notebook/browser/contrib/cellCommands/cellCommands.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ const COLLAPSE_ALL_CELL_OUTPUTS_COMMAND_ID = 'notebook.cell.collapseAllCellOutpu
357357
const EXPAND_ALL_CELL_OUTPUTS_COMMAND_ID = 'notebook.cell.expandAllCellOutputs';
358358
const TOGGLE_CELL_OUTPUTS_COMMAND_ID = 'notebook.cell.toggleOutputs';
359359
const TOGGLE_CELL_OUTPUT_SCROLLING = 'notebook.cell.toggleOutputScrolling';
360-
const OPEN_CELL_FAILURE_ACTIONS_COMMAND_ID = 'notebook.cell.openFailureActions';
360+
export const OPEN_CELL_FAILURE_ACTIONS_COMMAND_ID = 'notebook.cell.openFailureActions';
361361

362362
registerAction2(class CollapseCellInputAction extends NotebookMultiCellAction {
363363
constructor() {
@@ -601,7 +601,7 @@ registerAction2(class ExpandAllCellOutputsAction extends NotebookCellAction {
601601

602602
async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext): Promise<void> {
603603
if (context.cell instanceof CodeCellViewModel) {
604-
const error = context.cell.cellErrorDetails;
604+
const error = context.cell.cellDiagnostics.ErrorDetails;
605605
if (error?.location) {
606606
const location = Range.lift({
607607
startLineNumber: error.location.startLineNumber + 1,

src/vs/workbench/contrib/notebook/browser/contrib/cellDiagnostics/cellDiagnostics.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,14 @@ import { Iterable } from 'vs/base/common/iterator';
1212
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
1313
import { NotebookSetting } from 'vs/workbench/contrib/notebook/common/notebookCommon';
1414
import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel';
15+
import { Emitter, Event } from 'vs/base/common/event';
1516

1617

1718
export class CellDiagnostics extends Disposable {
1819

20+
private readonly _onDidDiagnosticsChange = new Emitter<void>();
21+
readonly onDidDiagnosticsChange: Event<void> = this._onDidDiagnosticsChange.event;
22+
1923
static ID: string = 'workbench.notebook.cellDiagnostics';
2024

2125
private enabled = false;
@@ -48,7 +52,7 @@ export class CellDiagnostics extends Disposable {
4852
const settingEnabled = this.configurationService.getValue(NotebookSetting.cellFailureDiagnostics);
4953
if (this.enabled && (!settingEnabled || Iterable.isEmpty(this.inlineChatService.getAllProvider()))) {
5054
this.enabled = false;
51-
this.clearDiagnostics();
55+
this.clear();
5256
} else if (!this.enabled && settingEnabled && !Iterable.isEmpty(this.inlineChatService.getAllProvider())) {
5357
this.enabled = true;
5458
if (!this.listening) {
@@ -62,7 +66,7 @@ export class CellDiagnostics extends Disposable {
6266
if (this.enabled && e.type === NotebookExecutionType.cell && e.affectsCell(this.cell.uri)) {
6367
if (!!e.changed) {
6468
// cell is running
65-
this.clearDiagnostics();
69+
this.clear();
6670
} else {
6771
this.setDiagnostics();
6872
}
@@ -71,21 +75,19 @@ export class CellDiagnostics extends Disposable {
7175

7276
public clear() {
7377
if (this.ErrorDetails) {
74-
this.clearDiagnostics();
78+
this.markerService.changeOne(CellDiagnostics.ID, this.cell.uri, []);
79+
this.errorDetails = undefined;
80+
this._onDidDiagnosticsChange.fire();
7581
}
7682
}
7783

78-
private clearDiagnostics() {
79-
this.markerService.changeOne(CellDiagnostics.ID, this.cell.uri, []);
80-
this.errorDetails = undefined;
81-
}
82-
8384
private setDiagnostics() {
8485
const metadata = this.cell.model.internalMetadata;
8586
if (!metadata.lastRunSuccess && metadata?.error?.location) {
8687
const marker = this.createMarkerData(metadata.error.message, metadata.error.location);
8788
this.markerService.changeOne(CellDiagnostics.ID, this.cell.uri, [marker]);
8889
this.errorDetails = metadata.error;
90+
this._onDidDiagnosticsChange.fire();
8991
}
9092
}
9193

src/vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ import { CellStatusbarAlignment, INotebookCellStatusBarItem, NotebookCellExecuti
1919
import { INotebookCellExecution, INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
2020
import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
2121
import { IMarkdownString } from 'vs/base/common/htmlContent';
22+
import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel';
23+
import { OPEN_CELL_FAILURE_ACTIONS_COMMAND_ID } from 'vs/workbench/contrib/notebook/browser/contrib/cellCommands/cellCommands';
24+
import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
2225

2326
export function formatCellDuration(duration: number, showMilliseconds: boolean = true): string {
2427
if (showMilliseconds && duration < 1000) {
@@ -333,3 +336,60 @@ class TimerCellStatusBarItem extends Disposable {
333336
this._notebookViewModel.deltaCellStatusBarItems(this._currentItemIds, [{ handle: this._cell.handle, items: [] }]);
334337
}
335338
}
339+
340+
export class DiagnosticCellStatusBarContrib extends Disposable implements INotebookEditorContribution {
341+
static id: string = 'workbench.notebook.statusBar.diagtnostic';
342+
343+
constructor(
344+
notebookEditor: INotebookEditor,
345+
@IInstantiationService instantiationService: IInstantiationService
346+
) {
347+
super();
348+
this._register(new NotebookStatusBarController(notebookEditor, (vm, cell) =>
349+
cell instanceof CodeCellViewModel ?
350+
instantiationService.createInstance(DiagnosticCellStatusBarItem, vm, cell) :
351+
Disposable.None
352+
));
353+
}
354+
}
355+
registerNotebookContribution(DiagnosticCellStatusBarContrib.id, DiagnosticCellStatusBarContrib);
356+
357+
358+
class DiagnosticCellStatusBarItem extends Disposable {
359+
private _currentItemIds: string[] = [];
360+
361+
constructor(
362+
private readonly _notebookViewModel: INotebookViewModel,
363+
private readonly cell: CodeCellViewModel,
364+
@IKeybindingService private readonly keybindingService: IKeybindingService
365+
) {
366+
super();
367+
this._update();
368+
this._register(this.cell.cellDiagnostics.onDidDiagnosticsChange(() => this._update()));
369+
}
370+
371+
private async _update() {
372+
let item: INotebookCellStatusBarItem | undefined;
373+
374+
if (!!this.cell.cellDiagnostics.ErrorDetails) {
375+
const keybinding = this.keybindingService.lookupKeybinding(OPEN_CELL_FAILURE_ACTIONS_COMMAND_ID)?.getLabel();
376+
const tooltip = localize('notebook.cell.status.diagnostic', "Quick Actions {0}", `(${keybinding})`);
377+
378+
item = {
379+
text: `$(sparkle)`,
380+
tooltip,
381+
alignment: CellStatusbarAlignment.Left,
382+
command: OPEN_CELL_FAILURE_ACTIONS_COMMAND_ID,
383+
priority: Number.MAX_SAFE_INTEGER - 1
384+
};
385+
}
386+
387+
const items = item ? [item] : [];
388+
this._currentItemIds = this._notebookViewModel.deltaCellStatusBarItems(this._currentItemIds, [{ handle: this.cell.handle, items }]);
389+
}
390+
391+
override dispose() {
392+
super.dispose();
393+
this._notebookViewModel.deltaCellStatusBarItems(this._currentItemIds, [{ handle: this.cell.handle, items: [] }]);
394+
}
395+
}

src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,9 +1101,9 @@ configurationRegistry.registerConfiguration({
11011101
default: false
11021102
},
11031103
[NotebookSetting.cellFailureDiagnostics]: {
1104-
markdownDescription: nls.localize('notebook.cellFailureDiagnostics', "Enable experimental diagnostics for cell failures."),
1104+
markdownDescription: nls.localize('notebook.cellFailureDiagnostics', "Show available diagnostics for cell failures."),
11051105
type: 'boolean',
1106-
default: false
1106+
default: true
11071107
},
11081108
}
11091109
});

src/vs/workbench/contrib/notebook/browser/view/cellParts/cellContextKeys.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ export class CellContextKeyManager extends Disposable {
101101

102102
if (element instanceof CodeCellViewModel) {
103103
this.elementDisposables.add(element.onDidChangeOutputs(() => this.updateForOutputs()));
104+
this.elementDisposables.add(element.cellDiagnostics.onDidDiagnosticsChange(() => this.updateForDiagnostics()));
104105
}
105106

106107
this.elementDisposables.add(this.notebookEditor.onDidChangeActiveCell(() => this.updateForFocusState()));
@@ -118,6 +119,7 @@ export class CellContextKeyManager extends Disposable {
118119
this.updateForCollapseState();
119120
this.updateForOutputs();
120121
this.updateForChat();
122+
this.updateForDiagnostics();
121123

122124
this.cellLineNumbers.set(this.element!.lineNumbers);
123125
this.cellResource.set(this.element!.uri.toString());
@@ -202,10 +204,6 @@ export class CellContextKeyManager extends Disposable {
202204
this.cellRunState.set('idle');
203205
this.cellExecuting.set(false);
204206
}
205-
206-
if (this.element instanceof CodeCellViewModel) {
207-
this.cellHasErrorDiagnostics.set(!!this.element.cellErrorDetails);
208-
}
209207
}
210208

211209
private updateForEditState() {
@@ -247,4 +245,10 @@ export class CellContextKeyManager extends Disposable {
247245

248246
this.cellGeneratedByChat.set(chatController.isCellGeneratedByChat(this.element));
249247
}
248+
249+
private updateForDiagnostics() {
250+
if (this.element instanceof CodeCellViewModel) {
251+
this.cellHasErrorDiagnostics.set(!!this.element.cellDiagnostics.ErrorDetails);
252+
}
253+
}
250254
}

src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod
4747
private _outputCollection: number[] = [];
4848

4949
private readonly _cellDiagnostics: CellDiagnostics;
50-
get cellErrorDetails() {
51-
return this._cellDiagnostics.ErrorDetails;
50+
get cellDiagnostics() {
51+
return this._cellDiagnostics;
5252
}
5353

5454
private _outputsTop: PrefixSumComputer | null = null;
@@ -436,10 +436,10 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod
436436

437437
protected onDidChangeTextModelContent(): void {
438438
if (this.getEditState() !== CellEditState.Editing) {
439-
this._cellDiagnostics.clear();
440439
this.updateEditState(CellEditState.Editing, 'onDidChangeTextModelContent');
441440
this._onDidChangeState.fire({ contentChanged: true });
442441
}
442+
this._cellDiagnostics.clear();
443443
}
444444

445445
onDeselect() {

src/vs/workbench/contrib/notebook/common/notebookCommon.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -956,7 +956,7 @@ export const NotebookSetting = {
956956
cellChat: 'notebook.experimental.cellChat',
957957
notebookVariablesView: 'notebook.experimental.variablesView',
958958
InteractiveWindowPromptToSave: 'interactiveWindow.promptToSaveOnClose',
959-
cellFailureDiagnostics: 'notebook.experimental.cellFailureDiagnostics',
959+
cellFailureDiagnostics: 'notebook.cellFailureDiagnostics',
960960
} as const;
961961

962962
export const enum CellStatusbarAlignment {

0 commit comments

Comments
 (0)