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.

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

easier to use show options, notebook repl editor helper #228577

Merged
merged 4 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,8 @@ export class MainThreadNotebooksAndEditors {
documentUri: add.textModel.uri,
selections: add.getSelections(),
visibleRanges: add.visibleRanges,
viewColumn: pane && editorGroupToColumn(this._editorGroupService, pane.group)
viewColumn: pane && editorGroupToColumn(this._editorGroupService, pane.group),
viewType: add.getViewModel().viewType
};
}
}
1 change: 1 addition & 0 deletions src/vs/workbench/api/common/extHost.protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2575,6 +2575,7 @@ export interface INotebookEditorAddData {
selections: ICellRange[];
visibleRanges: ICellRange[];
viewColumn?: number;
viewType: string;
}

export interface INotebookDocumentsAndEditorsDelta {
Expand Down
11 changes: 8 additions & 3 deletions src/vs/workbench/api/common/extHostNotebook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,11 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {
preserveFocus: options.preserveFocus,
selections: options.selections && options.selections.map(typeConverters.NotebookRange.from),
pinned: typeof options.preview === 'boolean' ? !options.preview : undefined,
label: options?.label
label: typeof options.asRepl === 'string' ?
options.asRepl :
typeof options.asRepl === 'object' ?
options.asRepl.label :
undefined,
};
} else {
resolvedOptions = {
Expand All @@ -220,7 +224,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {
};
}

const viewType = options?.asRepl ? 'repl' : notebook.notebookType;
const viewType = !!options?.asRepl ? 'repl' : notebook.notebookType;
const editorId = await this._notebookEditorsProxy.$tryShowNotebookDocument(notebook.uri, viewType, resolvedOptions);
const editor = editorId && this._editors.get(editorId)?.apiEditor;

Expand Down Expand Up @@ -588,7 +592,8 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {
document,
data.visibleRanges.map(typeConverters.NotebookRange.to),
data.selections.map(typeConverters.NotebookRange.to),
typeof data.viewColumn === 'number' ? typeConverters.ViewColumn.to(data.viewColumn) : undefined
typeof data.viewColumn === 'number' ? typeConverters.ViewColumn.to(data.viewColumn) : undefined,
data.viewType
);

this._editors.set(editorId, editor);
Expand Down
23 changes: 11 additions & 12 deletions src/vs/workbench/api/common/extHostNotebookEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ export class ExtHostNotebookEditor {

public static readonly apiEditorsToExtHost = new WeakMap<vscode.NotebookEditor, ExtHostNotebookEditor>();

private _selections: vscode.NotebookRange[] = [];
private _visibleRanges: vscode.NotebookRange[] = [];
private _viewColumn?: vscode.ViewColumn;

private _visible: boolean = false;

private _editor?: vscode.NotebookEditor;
Expand All @@ -26,14 +22,11 @@ export class ExtHostNotebookEditor {
readonly id: string,
private readonly _proxy: MainThreadNotebookEditorsShape,
readonly notebookData: ExtHostNotebookDocument,
visibleRanges: vscode.NotebookRange[],
selections: vscode.NotebookRange[],
viewColumn: vscode.ViewColumn | undefined
) {
this._selections = selections;
this._visibleRanges = visibleRanges;
this._viewColumn = viewColumn;
}
private _visibleRanges: vscode.NotebookRange[],
private _selections: vscode.NotebookRange[],
private _viewColumn: vscode.ViewColumn | undefined,
private readonly viewType: string
) { }

get apiEditor(): vscode.NotebookEditor {
if (!this._editor) {
Expand Down Expand Up @@ -71,6 +64,12 @@ export class ExtHostNotebookEditor {
get viewColumn() {
return that._viewColumn;
},
get replOptions() {
if (that.viewType === 'repl') {
return { appendIndex: this.notebook.cellCount - 1 };
}
return undefined;
},
[Symbol.for('debug.description')]() {
return `NotebookEditor(${this.notebook.uri.toString()})`;
}
Expand Down
6 changes: 4 additions & 2 deletions src/vs/workbench/api/test/browser/extHostNotebook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ suite('NotebookCell#Document', function () {
documentUri: notebookUri,
id: '_notebook_editor_0',
selections: [{ start: 0, end: 1 }],
visibleRanges: []
visibleRanges: [],
viewType: 'test'
}]
}));
extHostNotebooks.$acceptDocumentAndEditorsDelta(new SerializableObjectWithBuffers({ newActiveEditor: '_notebook_editor_0' }));
Expand Down Expand Up @@ -369,7 +370,8 @@ suite('NotebookCell#Document', function () {
documentUri: notebookUri,
id: '_notebook_editor_2',
selections: [{ start: 0, end: 1 }],
visibleRanges: []
visibleRanges: [],
viewType: 'test'
}]
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ suite('NotebookKernel', function () {
documentUri: notebookUri,
id: '_notebook_editor_0',
selections: [{ start: 0, end: 1 }],
visibleRanges: []
visibleRanges: [],
viewType: 'test',
}]
}));
extHostNotebooks.$acceptDocumentAndEditorsDelta(new SerializableObjectWithBuffers({ newActiveEditor: '_notebook_editor_0' }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,6 @@ export interface INotebookEditorCreationOptions {
};
readonly options?: NotebookOptions;
readonly codeWindow?: CodeWindow;
readonly forRepl?: boolean;
}

export interface INotebookWebviewMessage {
Expand Down Expand Up @@ -456,6 +455,7 @@ export interface INotebookViewModel {
notebookDocument: NotebookTextModel;
readonly viewCells: ICellViewModel[];
layoutInfo: NotebookLayoutInfo | null;
viewType: string;
onDidChangeViewCells: Event<INotebookViewCellsUpdateEvent>;
onDidChangeSelection: Event<string>;
onDidFoldingStateChanged: Event<void>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,6 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD

readonly isEmbedded: boolean;
private _readOnly: boolean;
private readonly _inRepl: boolean;

public readonly scopedContextKeyService: IContextKeyService;
private readonly instantiationService: IInstantiationService;
Expand Down Expand Up @@ -327,7 +326,6 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD

this.isEmbedded = creationOptions.isEmbedded ?? false;
this._readOnly = creationOptions.isReadOnly ?? false;
this._inRepl = creationOptions.forRepl ?? false;

this._overlayContainer = document.createElement('div');
this.scopedContextKeyService = this._register(contextKeyService.createScoped(this._overlayContainer));
Expand Down Expand Up @@ -1142,11 +1140,11 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
this.scopedContextKeyService.updateParent(parentContextKeyService);
}

async setModel(textModel: NotebookTextModel, viewState: INotebookEditorViewState | undefined, perf?: NotebookPerfMarks): Promise<void> {
async setModel(textModel: NotebookTextModel, viewState: INotebookEditorViewState | undefined, perf?: NotebookPerfMarks, viewType?: string): Promise<void> {
if (this.viewModel === undefined || !this.viewModel.equal(textModel)) {
const oldBottomToolbarDimensions = this._notebookOptions.computeBottomToolbarDimensions(this.viewModel?.viewType);
this._detachModel();
await this._attachModel(textModel, viewState, perf);
await this._attachModel(textModel, viewType ?? textModel.viewType, viewState, perf);
Copy link
Contributor Author

@amunger amunger Sep 13, 2024

Choose a reason for hiding this comment

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

more separation between the textModel's viewtype (more like notebook type) and the viewmodels viewtype - the notebook type is used by default, but a different viewtype could be set.
The only other usage of the viewmodel.viewtype was for determining the placement of the cell toolbar which can be configured by viewtype, and should therefore probably be independent from the notebook type

const newBottomToolbarDimensions = this._notebookOptions.computeBottomToolbarDimensions(this.viewModel?.viewType);

if (oldBottomToolbarDimensions.bottomToolbarGap !== newBottomToolbarDimensions.bottomToolbarGap
Expand Down Expand Up @@ -1452,10 +1450,10 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
this._list.attachWebview(this._webview.element);
}

private async _attachModel(textModel: NotebookTextModel, viewState: INotebookEditorViewState | undefined, perf?: NotebookPerfMarks) {
private async _attachModel(textModel: NotebookTextModel, viewType: string, viewState: INotebookEditorViewState | undefined, perf?: NotebookPerfMarks) {
this._ensureWebview(this.getId(), textModel.viewType, textModel.uri);

this.viewModel = this.instantiationService.createInstance(NotebookViewModel, textModel.viewType, textModel, this._viewContext, this.getLayoutInfo(), { isReadOnly: this._readOnly, inRepl: this._inRepl });
this.viewModel = this.instantiationService.createInstance(NotebookViewModel, viewType, textModel, this._viewContext, this.getLayoutInfo(), { isReadOnly: this._readOnly });
this._viewContext.eventDispatcher.emit([new NotebookLayoutChangedEvent({ width: true, fontInfo: true }, this.getLayoutInfo())]);
this.notebookOptions.updateOptions(this._readOnly);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ let MODEL_ID = 0;

export interface NotebookViewModelOptions {
isReadOnly: boolean;
inRepl?: boolean;
}

export class NotebookViewModel extends Disposable implements EditorFoldingStateDelegate, INotebookViewModel {
Expand All @@ -109,7 +108,6 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD
private readonly _onDidChangeOptions = this._register(new Emitter<void>());
get onDidChangeOptions(): Event<void> { return this._onDidChangeOptions.event; }
private _viewCells: CellViewModel[] = [];
private readonly replView: boolean;

get viewCells(): ICellViewModel[] {
return this._viewCells;
Expand All @@ -131,6 +129,10 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD
return this._notebook.metadata;
}

private get isRepl() {
return this.viewType === 'repl';
}

private readonly _onDidChangeViewCells = this._register(new Emitter<INotebookViewCellsUpdateEvent>());
get onDidChangeViewCells(): Event<INotebookViewCellsUpdateEvent> { return this._onDidChangeViewCells.event; }

Expand Down Expand Up @@ -204,7 +206,6 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD
MODEL_ID++;
this.id = '$notebookViewModel' + MODEL_ID;
this._instanceId = strings.singleLetterHash(MODEL_ID);
this.replView = !!this.options.inRepl;

const compute = (changes: NotebookCellTextModelSplice<ICell>[], synchronous: boolean) => {
const diffs = changes.map(splice => {
Expand Down Expand Up @@ -337,7 +338,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD
}));


const viewCellCount = this.replView ? this._notebook.cells.length - 1 : this._notebook.cells.length;
const viewCellCount = this.isRepl ? this._notebook.cells.length - 1 : this._notebook.cells.length;
for (let i = 0; i < viewCellCount; i++) {
this._viewCells.push(createCellViewModel(this._instantiationService, this, this._notebook.cells[i], this._viewContext));
}
Expand Down
3 changes: 1 addition & 2 deletions src/vs/workbench/contrib/replNotebook/browser/replEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,6 @@ export class ReplEditor extends EditorPane implements IEditorPaneWithScrolling {
this._notebookWidget = <IBorrowValue<NotebookEditorWidget>>this._instantiationService.invokeFunction(this._notebookWidgetService.retrieveWidget, this.group.id, input, {
isEmbedded: true,
isReadOnly: true,
forRepl: true,
contributions: NotebookEditorExtensionsRegistry.getSomeEditorContributions([
ExecutionStateCellStatusBarContrib.id,
TimerCellStatusBarContrib.id,
Expand Down Expand Up @@ -431,7 +430,7 @@ export class ReplEditor extends EditorPane implements IEditorPaneWithScrolling {

const viewState = options?.viewState ?? this._loadNotebookEditorViewState(input);
await this._extensionService.whenInstalledExtensionsRegistered();
await this._notebookWidget.value!.setModel(model.notebook, viewState?.notebook);
await this._notebookWidget.value!.setModel(model.notebook, viewState?.notebook, undefined, 'repl');
model.notebook.setCellCollapseDefault(this._notebookOptions.getCellCollapseDefault());
this._notebookWidget.value!.setOptions({
isReadOnly: true
Expand Down
21 changes: 17 additions & 4 deletions src/vscode-dts/vscode.proposed.notebookReplDocument.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,26 @@ declare module 'vscode' {
export interface NotebookDocumentShowOptions {
/**
* The notebook should be opened in a REPL editor,
* where the last cell of the notebook is an input box and the rest are read-only.
* where the last cell of the notebook is an input box and the other cells are the read-only history.
* When the value is a string, it will be used as the label for the editor tab.
*/
readonly asRepl?: boolean;
readonly asRepl?: boolean | string | {
/**
* The label to be used for the editor tab.
*/
readonly label: string;
};
}

export interface NotebookEditor {
/**
* The label to be used for the editor tab.
* Information about the REPL editor if the notebook was opened as a repl.
*/
readonly label?: string;
replOptions?: {
/**
* The index where new cells should be appended.
*/
appendIndex: number;
};
}
}
Loading