Don't clear execution order immediately when execution starts.

Same for the timer. Also fix spinner delay logic to correctly implement showing for a minimum time. Fixes #150924
microsoft · Jul 16, 2022 · 0e65b07 · 0e65b07
1 parent 1cd90cc
commit 0e65b07
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 19 deletions.
diff --git a/...kbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts b/...kbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts
@@ -93,7 +93,8 @@ class ExecutionStateCellStatusBarItem extends Disposable {
 
 	private _currentItemIds: string[] = [];
 
-	private _currentExecutingStateTimer: IDisposable | undefined;
+	private _showedExecutingStateTime: number | undefined;
+	private _clearExecutingStateTimer: IDisposable | undefined;
 
 	constructor(
 		private readonly _notebookViewModel: INotebookViewModel,
@@ -123,23 +124,27 @@ class ExecutionStateCellStatusBarItem extends Disposable {
 	 */
 	private _getItemsForCell(): INotebookCellStatusBarItem[] | undefined {
 		const runState = this._executionStateService.getCellExecution(this._cell.uri);
-		if (this._currentExecutingStateTimer && !runState?.isPaused) {
-			return;
-		}
-
-		const item = this._getItemForState(runState, this._cell.internalMetadata);
-
 		// Show the execution spinner for a minimum time
-		if (runState?.state === NotebookCellExecutionState.Executing) {
-			this._currentExecutingStateTimer = this._register(disposableTimeout(() => {
-				const runState = this._executionStateService.getCellExecution(this._cell.uri);
-				this._currentExecutingStateTimer = undefined;
-				if (runState?.state !== NotebookCellExecutionState.Executing) {
-					this._update();
+		if (runState?.state === NotebookCellExecutionState.Executing && typeof this._showedExecutingStateTime !== 'number') {
+			this._showedExecutingStateTime = Date.now();
+		} else if (runState?.state !== NotebookCellExecutionState.Executing && typeof this._showedExecutingStateTime === 'number') {
+			const timeUntilMin = ExecutionStateCellStatusBarItem.MIN_SPINNER_TIME - (Date.now() - this._showedExecutingStateTime);
+			if (timeUntilMin > 0) {
+				if (!this._clearExecutingStateTimer) {
+					this._clearExecutingStateTimer = disposableTimeout(() => {
+						this._showedExecutingStateTime = undefined;
+						this._clearExecutingStateTimer = undefined;
+						this._update();
+					}, timeUntilMin);
 				}
-			}, ExecutionStateCellStatusBarItem.MIN_SPINNER_TIME));
+
+				return undefined;
+			} else {
+				this._showedExecutingStateTime = undefined;
+			}
 		}
 
+		const item = this._getItemForState(runState, this._cell.internalMetadata);
 		return item ? [item] : [];
 	}
 
@@ -203,12 +208,16 @@ export class TimerCellStatusBarContrib extends Disposable implements INotebookEd
 }
 registerNotebookContribution(TimerCellStatusBarContrib.id, TimerCellStatusBarContrib);
 
+const UPDATE_TIMER_GRACE_PERIOD = 200;
+
 class TimerCellStatusBarItem extends Disposable {
 	private static UPDATE_INTERVAL = 100;
 	private _currentItemIds: string[] = [];
 
 	private _scheduler: RunOnceScheduler;
 
+	private _deferredUpdate: IDisposable | undefined;
+
 	constructor(
 		private readonly _notebookViewModel: INotebookViewModel,
 		private readonly _cell: ICellViewModel,
@@ -243,7 +252,18 @@ class TimerCellStatusBarItem extends Disposable {
 		}
 
 		const items = item ? [item] : [];
-		this._currentItemIds = this._notebookViewModel.deltaCellStatusBarItems(this._currentItemIds, [{ handle: this._cell.handle, items }]);
+		if (!items.length && !!runState) {
+			if (!this._deferredUpdate) {
+				this._deferredUpdate = disposableTimeout(() => {
+					this._deferredUpdate = undefined;
+					this._currentItemIds = this._notebookViewModel.deltaCellStatusBarItems(this._currentItemIds, [{ handle: this._cell.handle, items }]);
+				}, UPDATE_TIMER_GRACE_PERIOD);
+			}
+		} else {
+			this._deferredUpdate?.dispose();
+			this._deferredUpdate = undefined;
+			this._currentItemIds = this._notebookViewModel.deltaCellStatusBarItems(this._currentItemIds, [{ handle: this._cell.handle, items }]);
+		}
 	}
 
 	private _getTimeItem(startTime: number, endTime: number, adjustment: number = 0): INotebookCellStatusBarItem {
@@ -258,6 +278,7 @@ class TimerCellStatusBarItem extends Disposable {
 	override dispose() {
 		super.dispose();
 
+		this._deferredUpdate?.dispose();
 		this._notebookViewModel.deltaCellStatusBarItems(this._currentItemIds, [{ handle: this._cell.handle, items: [] }]);
 	}
 }
diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts
@@ -108,6 +108,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo
 		if (!exe) {
 			exe = this._createNotebookCellExecution(notebook, cellHandle);
 			notebookExecutionMap.set(cellHandle, exe);
+			exe.initialize();
 			this._onDidChangeCellExecution.fire(new NotebookExecutionEvent(notebookUri, cellHandle, exe));
 		}
 
@@ -279,6 +280,9 @@ class CellExecution extends Disposable implements INotebookCellExecution {
 	) {
 		super();
 		this._logService.debug(`CellExecution#ctor ${this.getCellLog()}`);
+	}
+
+	initialize() {
 		const startExecuteEdit: ICellEditOperation = {
 			editType: CellEditType.PartialInternalMetadata,
 			handle: this.cellHandle,

diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellExecution.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellExecution.ts
@@ -4,18 +4,23 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as DOM from 'vs/base/browser/dom';
+import { disposableTimeout } from 'vs/base/common/async';
 import { DisposableStore } from 'vs/base/common/lifecycle';
 import { ICellViewModel, INotebookEditorDelegate } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
 import { CellViewModelStateChangeEvent } from 'vs/workbench/contrib/notebook/browser/notebookViewEvents';
 import { CellPart } from 'vs/workbench/contrib/notebook/browser/view/cellPart';
 import { NotebookCellInternalMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon';
+import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
+
+const UPDATE_EXECUTION_ORDER_GRACE_PERIOD = 200;
 
 export class CellExecutionPart extends CellPart {
 	private kernelDisposables = this._register(new DisposableStore());
 
 	constructor(
 		private readonly _notebookEditor: INotebookEditorDelegate,
-		private readonly _executionOrderLabel: HTMLElement
+		private readonly _executionOrderLabel: HTMLElement,
+		@INotebookExecutionStateService private readonly _notebookExecutionStateService: INotebookExecutionStateService
 	) {
 		super();
 
@@ -37,11 +42,22 @@ export class CellExecutionPart extends CellPart {
 	}
 
 	protected override didRenderCell(element: ICellViewModel): void {
-		this.updateExecutionOrder(element.internalMetadata);
+		this.updateExecutionOrder(element.internalMetadata, true);
 	}
 
-	private updateExecutionOrder(internalMetadata: NotebookCellInternalMetadata): void {
+	private updateExecutionOrder(internalMetadata: NotebookCellInternalMetadata, forceClear = false): void {
 		if (this._notebookEditor.activeKernel?.implementsExecutionOrder) {
+			// If the executionOrder was just cleared, and the cell is executing, wait just a bit before clearing the view to avoid flashing
+			if (typeof internalMetadata.executionOrder !== 'number' && !forceClear && !!this._notebookExecutionStateService.getCellExecution(this.currentCell!.uri)) {
+				const renderingCell = this.currentCell;
+				this.cellDisposables.add(disposableTimeout(() => {
+					if (this.currentCell === renderingCell) {
+						this.updateExecutionOrder(this.currentCell!.internalMetadata, true);
+					}
+				}, UPDATE_EXECUTION_ORDER_GRACE_PERIOD));
+				return;
+			}
+
 			const executionOrderLabel = typeof internalMetadata.executionOrder === 'number' ?
 				`[${internalMetadata.executionOrder}]` :
 				'[ ]';

diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts
@@ -313,7 +313,7 @@ export class CodeCellRenderer extends AbstractCellRenderer implements IListRende
 			templateDisposables.add(scopedInstaService.createInstance(RunToolbar, this.notebookEditor, contextKeyService, container, runButtonContainer)),
 			templateDisposables.add(new CellDecorations(rootContainer, decorationContainer)),
 			templateDisposables.add(scopedInstaService.createInstance(CellComments, this.notebookEditor, cellCommentPartContainer)),
-			templateDisposables.add(new CellExecutionPart(this.notebookEditor, executionOrderLabel)),
+			templateDisposables.add(scopedInstaService.createInstance(CellExecutionPart, this.notebookEditor, executionOrderLabel)),
 			templateDisposables.add(scopedInstaService.createInstance(CollapsedCellOutput, this.notebookEditor, cellOutputCollapsedContainer)),
 			templateDisposables.add(new CollapsedCellInput(this.notebookEditor, cellInputCollapsedContainer)),
 			templateDisposables.add(new CellFocusPart(container, focusSinkElement, this.notebookEditor)),