Skip to content

Commit 9dbdd1f

Browse files
jriekenjrieken
committed
borrow notebook editors from a notebook widget service. let the service manage editor move and deal with group changes
1 parent 7130434 commit 9dbdd1f

5 files changed

Lines changed: 199 additions & 125 deletions

File tree

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

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,14 @@ import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookS
3232
import { NotebookService } from 'vs/workbench/contrib/notebook/browser/notebookServiceImpl';
3333
import { CellKind, CellUri, NotebookDocumentBackupData, NotebookEditorPriority } from 'vs/workbench/contrib/notebook/common/notebookCommon';
3434
import { NotebookProviderInfo } from 'vs/workbench/contrib/notebook/common/notebookProvider';
35-
import { IEditorGroup, OpenEditorContext, IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService';
35+
import { IEditorGroup } from 'vs/workbench/services/editor/common/editorGroupsService';
3636
import { IEditorService, IOpenEditorOverride } from 'vs/workbench/services/editor/common/editorService';
3737
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
3838
import { CustomEditorsAssociations, customEditorsAssociationsSettingId } from 'vs/workbench/services/editor/common/editorAssociationsSetting';
3939
import { CustomEditorInfo } from 'vs/workbench/contrib/customEditor/common/customEditor';
4040
import { NotebookEditorOptions } from 'vs/workbench/contrib/notebook/browser/notebookEditorWidget';
4141
import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
4242
import { IUndoRedoService } from 'vs/platform/undoRedo/common/undoRedo';
43-
import { NotebookRegistry } from 'vs/workbench/contrib/notebook/browser/notebookRegistry';
4443
import { INotebookEditorModelResolverService, NotebookModelResolverService } from 'vs/workbench/contrib/notebook/common/notebookEditorModelResolverService';
4544

4645
// Editor Contribution
@@ -146,7 +145,6 @@ export class NotebookContribution extends Disposable implements IWorkbenchContri
146145
@IInstantiationService private readonly instantiationService: IInstantiationService,
147146
@IConfigurationService private readonly configurationService: IConfigurationService,
148147
@IUndoRedoService undoRedoService: IUndoRedoService,
149-
@IEditorGroupsService private readonly editorGroupsService: IEditorGroupsService,
150148
) {
151149
super();
152150

@@ -189,24 +187,6 @@ export class NotebookContribution extends Disposable implements IWorkbenchContri
189187
}
190188
}));
191189

192-
// HACK
193-
// we use the open override to spy on tab movements because that's the only
194-
// way to do that...
195-
this._register(this.editorService.overrideOpenEditor({
196-
open: (input, _options, group, context) => {
197-
if (input instanceof NotebookEditorInput && context === OpenEditorContext.MOVE_EDITOR) {
198-
// when moving a notebook editor we release it from its current tab and we
199-
// "place" it into its future slot so that the editor can pick it up from there
200-
const widgetRef = NotebookRegistry.getNotebookEditorWidget(input.resource, this.editorGroupsService.activeGroup);
201-
if (widgetRef) {
202-
NotebookRegistry.releaseNotebookEditorWidget(input.resource, this.editorGroupsService.activeGroup);
203-
NotebookRegistry.claimNotebookEditorWidget(input.resource, group, widgetRef);
204-
}
205-
}
206-
return undefined;
207-
}
208-
}));
209-
210190
this._register(this.editorService.onDidVisibleEditorsChange(() => {
211191
const visibleNotebookEditors = editorService.visibleEditorPanes
212192
.filter(pane => (pane as any).isNotebookEditor)

‎src/vs/workbench/contrib/notebook/browser/notebookEditor.ts‎

Lines changed: 39 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,20 @@ import { NotebookEditorInput } from 'vs/workbench/contrib/notebook/browser/noteb
1717
import { INotebookEditorViewState, NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel';
1818
import { IEditorGroup, IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService';
1919
import { NotebookEditorWidget } from 'vs/workbench/contrib/notebook/browser/notebookEditorWidget';
20-
import { NotebookRegistry } from 'vs/workbench/contrib/notebook/browser/notebookRegistry';
2120
import { EditorPart } from 'vs/workbench/browser/parts/editor/editorPart';
2221
import { INotificationService, Severity } from 'vs/platform/notification/common/notification';
2322
import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
2423
import { IEditorOptions, ITextEditorOptions } from 'vs/platform/editor/common/editor';
24+
import { INotebookEditorWidgetService, IBorrowValue } from 'vs/workbench/contrib/notebook/browser/notebookEditorWidgetService';
25+
import { localize } from 'vs/nls';
2526

2627
const NOTEBOOK_EDITOR_VIEW_STATE_PREFERENCE_KEY = 'NotebookEditorViewState';
2728

2829
export class NotebookEditor extends BaseEditor {
2930
static readonly ID: string = 'workbench.editor.notebook';
3031
private editorMemento: IEditorMemento<INotebookEditorViewState>;
3132
private readonly groupListener = this._register(new MutableDisposable());
32-
private _widget?: NotebookEditorWidget;
33+
private _widget: IBorrowValue<NotebookEditorWidget> = { value: undefined };
3334
private _rootElement!: HTMLElement;
3435
private dimension: DOM.Dimension | null = null;
3536
private _widgetDisposableStore: DisposableStore = new DisposableStore();
@@ -43,7 +44,8 @@ export class NotebookEditor extends BaseEditor {
4344
@IStorageService storageService: IStorageService,
4445
@IEditorService private readonly editorService: IEditorService,
4546
@IEditorGroupsService private readonly editorGroupService: IEditorGroupsService,
46-
@INotificationService private readonly notificationService: INotificationService
47+
@INotificationService private readonly notificationService: INotificationService,
48+
@INotebookEditorWidgetService private readonly notebookWidgetService: INotebookEditorWidgetService,
4749
) {
4850
super(NotebookEditor.ID, telemetryService, themeService, storageService);
4951
this.editorMemento = this.getEditorMemento<INotebookEditorViewState>(editorGroupService, NOTEBOOK_EDITOR_VIEW_STATE_PREFERENCE_KEY);
@@ -54,14 +56,14 @@ export class NotebookEditor extends BaseEditor {
5456

5557

5658
set viewModel(newModel: NotebookViewModel | undefined) {
57-
if (this._widget) {
58-
this._widget.viewModel = newModel;
59+
if (this._widget.value) {
60+
this._widget.value.viewModel = newModel;
5961
this._onDidChangeModel.fire();
6062
}
6163
}
6264

6365
get viewModel() {
64-
return this._widget?.viewModel;
66+
return this._widget.value?.viewModel;
6567
}
6668

6769
get minimumWidth(): number { return 375; }
@@ -83,28 +85,26 @@ export class NotebookEditor extends BaseEditor {
8385
this._rootElement = DOM.append(parent, DOM.$('.notebook-editor'));
8486

8587
// this._widget.createEditor();
86-
this._register(this.onDidFocus(() => this._widget?.updateEditorFocus()));
87-
this._register(this.onDidBlur(() => this._widget?.updateEditorFocus()));
88+
this._register(this.onDidFocus(() => this._widget.value?.updateEditorFocus()));
89+
this._register(this.onDidBlur(() => this._widget.value?.updateEditorFocus()));
8890
}
8991

9092
getDomNode() {
9193
return this._rootElement;
9294
}
9395

94-
getControl() {
95-
return this._widget;
96+
getControl(): NotebookEditorWidget | undefined {
97+
return this._widget.value;
9698
}
9799

98100
onWillHide() {
99-
if (this.input && this.input instanceof NotebookEditorInput && !this.input.isDisposed()) {
101+
if (this.input instanceof NotebookEditorInput) {
100102
this.saveEditorViewState(this.input);
101103
}
102-
103-
if (this.input && NotebookRegistry.getNotebookEditorWidget(this.input.resource!, this.group!) === this._widget) {
104+
if (this.input && this._widget.value) {
104105
// the widget is not transfered to other editor inputs
105-
this._widget?.onWillHide();
106+
this._widget.value.onWillHide();
106107
}
107-
108108
super.onWillHide();
109109
}
110110

@@ -126,71 +126,42 @@ export class NotebookEditor extends BaseEditor {
126126

127127
focus() {
128128
super.focus();
129-
this._widget?.focus();
129+
this._widget.value?.focus();
130130
}
131131

132132
async setInput(input: NotebookEditorInput, options: EditorOptions | undefined, token: CancellationToken): Promise<void> {
133133

134134
const group = this.group!;
135135

136136
if (this.input instanceof NotebookEditorInput) {
137-
if (!this.input.isDisposed()) {
138-
// set a new input, let's hide previous input
139-
this.saveEditorViewState(this.input as NotebookEditorInput);
140-
this._widget?.onWillHide();
141-
}
137+
// set a new input, let's hide previous input
138+
this.saveEditorViewState(this.input as NotebookEditorInput);
142139
}
143140

144141
await super.setInput(input, options, token);
145142

146-
147-
// input attached
148-
Event.once(input.onDispose)(() => {
149-
// todo@jrieken
150-
// there is no more input for a resource and that is used as signal to clean up
151-
// all widget that might still be around
152-
for (let widget of NotebookRegistry.releaseAllNotebookEditorWidgets(input.resource)) {
153-
widget.onWillHide();
154-
widget.getDomNode().remove();
155-
widget.dispose();
156-
}
157-
});
158-
159143
this._widgetDisposableStore.clear();
160144

161-
const existingEditorWidgetForInput = NotebookRegistry.getNotebookEditorWidget(input.resource, group);
162-
if (existingEditorWidgetForInput) {
163-
// hide previous widget
164-
if (NotebookRegistry.getNotebookEditorWidget(this.input!.resource!, this.group!) === this._widget) {
165-
// the widet is not transfered to other editor inputs
166-
this._widget?.onWillHide();
167-
}
168-
169-
// previous widget is then detached
170-
// set the new one
171-
this._widget = existingEditorWidgetForInput;
172-
NotebookRegistry.claimNotebookEditorWidget(input.resource, group, this._widget);
173-
} else {
174-
// hide current widget
175-
this._widget?.onWillHide();
176-
// create a new widget
177-
this._widget = this.instantiationService.createInstance(NotebookEditorWidget);
178-
this._widget.createEditor();
179-
NotebookRegistry.claimNotebookEditorWidget(input.resource, group, this._widget);
145+
// there currently is a widget which we still own so
146+
// we need to hide it before getting a new widget
147+
if (this._widget.value) {
148+
this._widget.value.onWillHide();
180149
}
181150

151+
this._widget = this.instantiationService.invokeFunction(this.notebookWidgetService.retrieveWidget, group, input);
152+
182153
if (this.dimension) {
183-
this._widget.layout(this.dimension, this._rootElement);
154+
this._widget.value!.layout(this.dimension, this._rootElement);
184155
}
185156

186-
const model = await input.resolve(this._widget!.getId());
157+
const model = await input.resolve(this._widget.value!.getId());
187158

188159
if (model === null) {
189160
this.notificationService.prompt(
190161
Severity.Error,
191-
`Cannot open resource with notebook editor type '${input.viewType}', please check if you have the right extension installed or enabled.`,
162+
localize('fail.noEditor', "Cannot open resource with notebook editor type '${input.viewType}', please check if you have the right extension installed or enabled."),
192163
[{
193-
label: 'Reopen file with VS Code standard text editor',
164+
label: localize('fail.reOpen', "Reopen file with VS Code standard text editor"),
194165
run: async () => {
195166
const fileEditorInput = this.editorService.createEditorInput({ resource: input.resource, forceFile: true });
196167
const textOptions: IEditorOptions | ITextEditorOptions = options ? { ...options, override: false } : { override: false };
@@ -203,26 +174,26 @@ export class NotebookEditor extends BaseEditor {
203174

204175
const viewState = this.loadTextEditorViewState(input);
205176

206-
await this._widget.setModel(model.notebook, viewState, options);
207-
this._widgetDisposableStore.add(this._widget.onDidFocus(() => this._onDidFocusWidget.fire()));
177+
await this._widget.value!.setModel(model.notebook, viewState, options);
178+
this._widgetDisposableStore.add(this._widget.value!.onDidFocus(() => this._onDidFocusWidget.fire()));
208179

209180
if (this.editorGroupService instanceof EditorPart) {
210-
this._widgetDisposableStore.add(this.editorGroupService.createEditorDropTarget(this._widget.getDomNode(), {
181+
this._widgetDisposableStore.add(this.editorGroupService.createEditorDropTarget(this._widget.value!.getDomNode(), {
211182
groupContainsPredicate: (group) => this.group?.id === group.group.id
212183
}));
213184
}
214185
}
215186

216187
clearInput(): void {
217-
const existingEditorWidgetForInput = NotebookRegistry.getNotebookEditorWidget(this.input!.resource!, this.group!);
218-
existingEditorWidgetForInput?.onWillHide();
219-
this._widget = undefined;
188+
if (this._widget.value) {
189+
this._widget.value.onWillHide();
190+
}
220191
super.clearInput();
221192
}
222193

223194
private saveEditorViewState(input: NotebookEditorInput): void {
224-
if (this.group && this._widget) {
225-
const state = this._widget.getEditorViewState();
195+
if (this.group && this._widget.value) {
196+
const state = this._widget.value.getEditorViewState();
226197
this.editorMemento.saveEditorState(this.group, input.resource, state);
227198
}
228199
}
@@ -240,11 +211,11 @@ export class NotebookEditor extends BaseEditor {
240211
DOM.toggleClass(this._rootElement, 'narrow-width', dimension.width < 600);
241212
this.dimension = dimension;
242213

243-
if (this._input === undefined || this._widget === undefined) {
214+
if (!this._widget.value || !(this._input instanceof NotebookEditorInput)) {
244215
return;
245216
}
246217

247-
if (this._input.resource?.toString() !== this._widget?.viewModel?.uri.toString() && this._widget?.viewModel) {
218+
if (this._input.resource.toString() !== this._widget.value.viewModel?.uri.toString() && this._widget.value?.viewModel) {
248219
// input and widget mismatch
249220
// this happens when
250221
// 1. open document A, pin the document
@@ -254,7 +225,7 @@ export class NotebookEditor extends BaseEditor {
254225
return;
255226
}
256227

257-
this._widget?.layout(this.dimension, this._rootElement);
228+
this._widget.value.layout(this.dimension, this._rootElement);
258229
}
259230

260231
protected saveState(): void {

‎src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor
9898
public get onDidFocus(): Event<any> { return this._onDidFocusWidget.event; }
9999
private _cellContextKeyManager: CellContextKeyManager | null = null;
100100
private _isVisible = false;
101+
private readonly _uuid = generateUuid();
101102

102103
get isDisposed() {
103104
return this._isDisposed;
@@ -130,7 +131,6 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor
130131
this.notebookService.addNotebookEditor(this);
131132
}
132133

133-
private _uuid = generateUuid();
134134
public getId(): string {
135135
return this._uuid;
136136
}

0 commit comments

Comments
 (0)