Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add notebook Format On Cell Execution #182924

Merged
merged 5 commits into from May 22, 2023
Merged

Add notebook Format On Cell Execution #182924

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
868d104
add notebook Format On Cell Execution
Yoyokrazy May 18, 2023
8b62c26
Merge branch 'main' into milively/formatCellOnRun
Yoyokrazy May 19, 2023
b30038b
+cellExecutionParticipant, +formatOnCellExecution support, ~rename se…
Yoyokrazy May 19, 2023
8bedce9
Merge branch 'main' into milively/formatCellOnRun
Yoyokrazy May 22, 2023
dec9d99
refactor OUT of ExecStateService INTO ExecService
Yoyokrazy May 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/vs/workbench/contrib/notebook/browser/contrib/format/formatting.ts
View file
Open in desktop
Expand Up @@ -24,7 +24,8 @@ import { NOTEBOOK_ACTIONS_CATEGORY } from 'vs/workbench/contrib/notebook/browser
import { getNotebookEditorFromEditorPane } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import { NOTEBOOK_EDITOR_EDITABLE, NOTEBOOK_IS_ACTIVE_EDITOR } from 'vs/workbench/contrib/notebook/common/notebookContextKeys';
import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
import { ICellExecutionParticipant, INotebookCellExecution, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
import { INotebookCellExecution } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
import { ICellExecutionParticipant, INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService';
import { NotebookSetting } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle';
Expand Down Expand Up @@ -206,14 +207,14 @@ class FormatOnCellExecutionParticipant implements ICellExecutionParticipant {
export class CellExecutionParticipantsContribution extends Disposable implements IWorkbenchContribution {
constructor(
@IInstantiationService private readonly instantiationService: IInstantiationService,
@INotebookExecutionStateService private readonly notebookExecutionStateService: INotebookExecutionStateService
@INotebookExecutionService private readonly notebookExecutionService: INotebookExecutionService
) {
super();
this.registerKernelExecutionParticipants();
}

private registerKernelExecutionParticipants(): void {
this._register(this.notebookExecutionStateService.registerExecutionParticipant(this.instantiationService.createInstance(FormatOnCellExecutionParticipant)));
this._register(this.notebookExecutionService.registerExecutionParticipant(this.instantiationService.createInstance(FormatOnCellExecutionParticipant)));
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts
View file
Open in desktop
Expand Up @@ -926,13 +926,11 @@ configurationRegistry.registerConfiguration({
additionalProperties: {
type: 'boolean'
},
tags: ['notebookLayout'],
default: {}
},
[NotebookSetting.formatOnCellExecution]: {
markdownDescription: nls.localize('notebook.formatOnCellExecution', "Format a notebook cell upon execution. A formatter must be available, and the editor must not be shutting down."),
markdownDescription: nls.localize('notebook.formatOnCellExecution', "Format a notebook cell upon execution. A formatter must be available."),
type: 'boolean',
tags: ['notebookLayout'],
default: false
},
[NotebookSetting.confirmDeleteRunningCell]: {
Expand Down
20 changes: 17 additions & 3 deletions src/vs/workbench/contrib/notebook/browser/services/notebookExecutionServiceImpl.ts
View file
Open in desktop
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { CancellationTokenSource } from 'vs/base/common/cancellation';
import { IDisposable } from 'vs/base/common/lifecycle';
import { IDisposable, toDisposable } from 'vs/base/common/lifecycle';
import * as nls from 'vs/nls';
import { ICommandService } from 'vs/platform/commands/common/commands';
import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
Expand All @@ -13,7 +13,7 @@ import { IWorkspaceTrustRequestService } from 'vs/platform/workspace/common/work
import { KernelPickerMRUStrategy } from 'vs/workbench/contrib/notebook/browser/viewParts/notebookKernelQuickPickStrategy';
import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel';
import { CellKind, INotebookTextModel, NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService';
import { INotebookExecutionService, ICellExecutionParticipant } from 'vs/workbench/contrib/notebook/common/notebookExecutionService';
import { INotebookCellExecution, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
import { INotebookKernelHistoryService, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService';

Expand Down Expand Up @@ -78,7 +78,7 @@ export class NotebookExecutionService implements INotebookExecutionService, IDis

// request execution
if (validCellExecutions.length > 0) {
await this._notebookExecutionStateService.runExecutionParticipants(validCellExecutions);
await this.runExecutionParticipants(validCellExecutions);

this._notebookKernelService.selectKernelForNotebook(kernel, notebook);
await kernel.executeNotebookCellsRequest(notebook.uri, validCellExecutions.map(c => c.cellHandle));
Expand All @@ -105,6 +105,20 @@ export class NotebookExecutionService implements INotebookExecutionService, IDis
this.cancelNotebookCellHandles(notebook, Array.from(cells, cell => cell.handle));
}

private readonly cellExecutionParticipants = new Set<ICellExecutionParticipant>;

registerExecutionParticipant(participant: ICellExecutionParticipant) {
this.cellExecutionParticipants.add(participant);
return toDisposable(() => this.cellExecutionParticipants.delete(participant));
}

async runExecutionParticipants(executions: INotebookCellExecution[]): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be private unless there's a reason to expose it publicly

Copy link
Contributor Author

@Yoyokrazy Yoyokrazy May 22, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by: 183164, missed your point on the first read through. Appreciate the help and review through this.

for (const participant of this.cellExecutionParticipants) {
await participant.onWillExecuteCell(executions);
}
return;
}

dispose() {
this._activeProxyKernelExecutionToken?.dispose(true);
}
Expand Down
19 changes: 2 additions & 17 deletions src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts
View file
Open in desktop
Expand Up @@ -4,20 +4,19 @@
*--------------------------------------------------------------------------------------------*/

import { Emitter } from 'vs/base/common/event';
import { combinedDisposable, Disposable, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { combinedDisposable, Disposable, IDisposable } from 'vs/base/common/lifecycle';
import { ResourceMap } from 'vs/base/common/map';
import { isEqual } from 'vs/base/common/resources';
import { withNullAsUndefined } from 'vs/base/common/types';
import { URI } from 'vs/base/common/uri';
import { insert } from 'vs/base/common/arrays';
import { generateUuid } from 'vs/base/common/uuid';
import { AudioCue, IAudioCueService } from 'vs/platform/audioCues/browser/audioCueService';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { ILogService } from 'vs/platform/log/common/log';
import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel';
import { CellEditType, CellUri, ICellEditOperation, NotebookCellExecutionState, NotebookCellInternalMetadata, NotebookExecutionState, NotebookTextModelWillAddRemoveEvent } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { CellExecutionUpdateType, INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService';
import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionStateChangedEvent, ICellExecutionStateUpdate, IExecutionStateChangedEvent, IFailedCellInfo, ICellExecutionParticipant, INotebookCellExecution, INotebookExecution, INotebookExecutionStateService, INotebookFailStateChangedEvent, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionStateChangedEvent, ICellExecutionStateUpdate, IExecutionStateChangedEvent, IFailedCellInfo, INotebookCellExecution, INotebookExecution, INotebookExecutionStateService, INotebookFailStateChangedEvent, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService';
import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';

Expand Down Expand Up @@ -45,20 +44,6 @@ export class NotebookExecutionStateService extends Disposable implements INotebo
super();
}

private readonly cellExecutionParticipants: ICellExecutionParticipant[] = [];

registerExecutionParticipant(participant: ICellExecutionParticipant) {
const remove = insert(this.cellExecutionParticipants, participant);
return toDisposable(() => remove());
}

async runExecutionParticipants(executions: INotebookCellExecution[]): Promise<void> {
for (const participant of this.cellExecutionParticipants) {
await participant.onWillExecuteCell(executions);
}
return;
}

getLastFailedCellForNotebook(notebook: URI): number | undefined {
const failedCell = this._lastFailedCells.get(notebook);
return failedCell?.visible ? failedCell.cellHandle : undefined;
Expand Down
8 changes: 8 additions & 0 deletions src/vs/workbench/contrib/notebook/common/notebookExecutionService.ts
View file
Open in desktop
Expand Up @@ -3,10 +3,12 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { IDisposable } from 'vs/base/common/lifecycle';
import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel';
import { INotebookTextModel, IOutputDto, IOutputItemDto } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { INotebookCellExecution } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';

export enum CellExecutionUpdateType {
Output = 1,
Expand Down Expand Up @@ -36,4 +38,10 @@ export interface INotebookExecutionService {
executeNotebookCells(notebook: INotebookTextModel, cells: Iterable<NotebookCellTextModel>, contextKeyService: IContextKeyService): Promise<void>;
cancelNotebookCells(notebook: INotebookTextModel, cells: Iterable<NotebookCellTextModel>): Promise<void>;
cancelNotebookCellHandles(notebook: INotebookTextModel, cells: Iterable<number>): Promise<void>;
registerExecutionParticipant(participant: ICellExecutionParticipant): IDisposable;
runExecutionParticipants(executions: INotebookCellExecution[]): Promise<void>;
}

export interface ICellExecutionParticipant {
onWillExecuteCell(executions: INotebookCellExecution[]): Promise<void>;
}
5 changes: 0 additions & 5 deletions src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts
View file
Open in desktop
Expand Up @@ -36,9 +36,6 @@ export interface ICellExecutionStateChangedEvent {
affectsCell(cell: URI): boolean;
affectsNotebook(notebook: URI): boolean;
}
export interface ICellExecutionParticipant {
onWillExecuteCell(executions: INotebookCellExecution[]): Promise<void>;
}
export interface IExecutionStateChangedEvent {
type: NotebookExecutionType.notebook;
notebook: URI;
Expand Down Expand Up @@ -72,8 +69,6 @@ export interface INotebookExecutionStateService {
getExecution(notebook: URI): INotebookExecution | undefined;
createExecution(notebook: URI): INotebookExecution;
getLastFailedCellForNotebook(notebook: URI): number | undefined;
registerExecutionParticipant(participant: ICellExecutionParticipant): IDisposable;
runExecutionParticipants(executions: INotebookCellExecution[]): Promise<void>;
}

export interface INotebookCellExecution {
Expand Down