Skip to content

Commit f14fa4d

Browse files
jriekenjrieken
authored
Merge pull request microsoft#100266 from microsoft/joh/notebook-inputs
Ref-count notebook models, rework notebook inputs
2 parents bb55416 + c98d54e commit f14fa4d

12 files changed

Lines changed: 293 additions & 408 deletions

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

Lines changed: 47 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@
66
import { coalesce, distinct } from 'vs/base/common/arrays';
77
import { Schemas } from 'vs/base/common/network';
88
import { IDisposable, Disposable } from 'vs/base/common/lifecycle';
9-
import { ResourceMap } from 'vs/base/common/map';
109
import { parse } from 'vs/base/common/marshalling';
11-
import { basename, isEqual } from 'vs/base/common/resources';
10+
import { isEqual } from 'vs/base/common/resources';
1211
import { assertType } from 'vs/base/common/types';
1312
import { URI } from 'vs/base/common/uri';
1413
import { ITextModel, ITextBufferFactory, DefaultEndOfLine, ITextBuffer } from 'vs/editor/common/model';
@@ -17,7 +16,7 @@ import { IModeService } from 'vs/editor/common/services/modeService';
1716
import { ITextModelContentProvider, ITextModelService } from 'vs/editor/common/services/resolverService';
1817
import * as nls from 'vs/nls';
1918
import { Extensions, IConfigurationRegistry } from 'vs/platform/configuration/common/configurationRegistry';
20-
import { IEditorOptions, ITextEditorOptions } from 'vs/platform/editor/common/editor';
19+
import { IEditorOptions, ITextEditorOptions, IResourceEditorInput } from 'vs/platform/editor/common/editor';
2120
import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
2221
import { registerSingleton } from 'vs/platform/instantiation/common/extensions';
2322
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
@@ -33,15 +32,14 @@ import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookS
3332
import { NotebookService } from 'vs/workbench/contrib/notebook/browser/notebookServiceImpl';
3433
import { CellKind, CellUri, NotebookDocumentBackupData, NotebookEditorPriority } from 'vs/workbench/contrib/notebook/common/notebookCommon';
3534
import { NotebookProviderInfo } from 'vs/workbench/contrib/notebook/common/notebookProvider';
36-
import { IEditorGroup, OpenEditorContext } from 'vs/workbench/services/editor/common/editorGroupsService';
35+
import { IEditorGroup } from 'vs/workbench/services/editor/common/editorGroupsService';
3736
import { IEditorService, IOpenEditorOverride } from 'vs/workbench/services/editor/common/editorService';
3837
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
3938
import { CustomEditorsAssociations, customEditorsAssociationsSettingId } from 'vs/workbench/services/editor/common/editorAssociationsSetting';
4039
import { CustomEditorInfo } from 'vs/workbench/contrib/customEditor/common/customEditor';
4140
import { NotebookEditorOptions } from 'vs/workbench/contrib/notebook/browser/notebookEditorWidget';
4241
import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
4342
import { IUndoRedoService } from 'vs/platform/undoRedo/common/undoRedo';
44-
import { NotebookRegistry } from 'vs/workbench/contrib/notebook/browser/notebookRegistry';
4543
import { INotebookEditorModelResolverService, NotebookModelResolverService } from 'vs/workbench/contrib/notebook/common/notebookEditorModelResolverService';
4644

4745
// Editor Contribution
@@ -83,7 +81,6 @@ class NotebookEditorFactory implements IEditorInputFactory {
8381
resource: input.resource,
8482
name: input.name,
8583
viewType: input.viewType,
86-
group: input.group
8784
});
8885
}
8986
deserialize(instantiationService: IInstantiationService, raw: string) {
@@ -97,13 +94,7 @@ class NotebookEditorFactory implements IEditorInputFactory {
9794
return undefined;
9895
}
9996

100-
// if we have two editors open with the same resource (in different editor groups), we should then create two different
101-
// editor inputs, instead of `getOrCreate`.
10297
const input = NotebookEditorInput.create(instantiationService, resource, name, viewType);
103-
if (typeof data.group === 'number') {
104-
input.updateGroup(data.group);
105-
}
106-
10798
return input;
10899
}
109100

@@ -147,14 +138,13 @@ function getFirstNotebookInfo(notebookService: INotebookService, uri: URI): Note
147138
}
148139

149140
export class NotebookContribution extends Disposable implements IWorkbenchContribution {
150-
private _resourceMapping = new ResourceMap<NotebookEditorInput>();
151141

152142
constructor(
153143
@IEditorService private readonly editorService: IEditorService,
154144
@INotebookService private readonly notebookService: INotebookService,
155145
@IInstantiationService private readonly instantiationService: IInstantiationService,
156146
@IConfigurationService private readonly configurationService: IConfigurationService,
157-
@IUndoRedoService undoRedoService: IUndoRedoService
147+
@IUndoRedoService undoRedoService: IUndoRedoService,
158148
) {
159149
super();
160150

@@ -192,7 +182,9 @@ export class NotebookContribution extends Disposable implements IWorkbenchContri
192182
};
193183
});
194184
},
195-
open: (editor, options, group, context) => this.onEditorOpening(editor, options, group, context)
185+
open: (editor, options, group) => {
186+
return this.onEditorOpening2(editor, options, group);
187+
}
196188
}));
197189

198190
this._register(this.editorService.onDidVisibleEditorsChange(() => {
@@ -214,22 +206,6 @@ export class NotebookContribution extends Disposable implements IWorkbenchContri
214206
this.notebookService.updateActiveNotebookEditor(null);
215207
}
216208
}));
217-
218-
this._register(this.editorService.onDidCloseEditor(({ editor }) => {
219-
if (!(editor instanceof NotebookEditorInput)) {
220-
return;
221-
}
222-
223-
if (!this.editorService.editors.some(other => (
224-
other.resource === editor.resource
225-
&& other instanceof NotebookEditorInput
226-
&& other.viewType === editor.viewType
227-
))) {
228-
editor.clearTextModel();
229-
}
230-
231-
editor.dispose();
232-
}));
233209
}
234210

235211
getUserAssociatedEditors(resource: URI) {
@@ -251,54 +227,38 @@ export class NotebookContribution extends Disposable implements IWorkbenchContri
251227
return this.notebookService.getContributedNotebookProviders(resource);
252228
}
253229

254-
private onEditorOpening(originalInput: IEditorInput, options: IEditorOptions | ITextEditorOptions | undefined, group: IEditorGroup, context: OpenEditorContext): IOpenEditorOverride | undefined {
230+
private onEditorOpening2(originalInput: IEditorInput, options: IEditorOptions | ITextEditorOptions | undefined, group: IEditorGroup): IOpenEditorOverride | undefined {
231+
255232
const id = typeof options?.override === 'string' ? options.override : undefined;
256233
if (id === undefined && originalInput.isUntitled()) {
257-
return;
234+
return undefined;
258235
}
259236

260-
if (originalInput instanceof NotebookEditorInput) {
261-
if ((originalInput.group === group.id || originalInput.group === undefined) && (originalInput.viewType === id || typeof id !== 'string')) {
262-
// No need to do anything
263-
originalInput.updateGroup(group.id);
264-
return {
265-
override: this.editorService.openEditor(originalInput, new NotebookEditorOptions(options || {}).with({ override: false }), group)
266-
};
267-
} else {
268-
// Create a copy of the input.
269-
// Unlike normal editor inputs, we do not want to share custom editor inputs
270-
// between multiple editors / groups.
271-
const copiedInput = NotebookEditorInput.create(this.instantiationService, originalInput.resource, originalInput.name, originalInput.viewType);
272-
copiedInput.updateGroup(group.id);
273-
274-
if (context === OpenEditorContext.MOVE_EDITOR) {
275-
// transfer ownership of editor widget
276-
const widgetRef = NotebookRegistry.getNotebookEditorWidget(originalInput);
277-
if (widgetRef) {
278-
NotebookRegistry.releaseNotebookEditorWidget(originalInput);
279-
NotebookRegistry.claimNotebookEditorWidget(copiedInput, widgetRef);
280-
}
281-
}
282-
283-
return {
284-
override: this.editorService.openEditor(copiedInput, new NotebookEditorOptions(options || {}).with({ override: false }), group)
285-
};
286-
}
237+
if (!originalInput.resource) {
238+
return undefined;
287239
}
288240

289-
let resource = originalInput.resource;
290-
if (!resource) {
241+
if (originalInput instanceof NotebookEditorInput) {
291242
return undefined;
292243
}
293244

245+
let notebookUri: URI = originalInput.resource;
246+
let cellOptions: IResourceEditorInput | undefined;
247+
248+
const data = CellUri.parse(originalInput.resource);
249+
if (data) {
250+
notebookUri = data.notebook;
251+
cellOptions = { resource: originalInput.resource, options };
252+
}
253+
294254
if (id === undefined) {
295-
const existingEditors = group.editors.filter(editor => editor.resource && isEqual(editor.resource, resource) && !(editor instanceof NotebookEditorInput));
255+
const existingEditors = group.editors.filter(editor => editor.resource && isEqual(editor.resource, notebookUri) && !(editor instanceof NotebookEditorInput));
296256

297257
if (existingEditors.length) {
298258
return undefined;
299259
}
300260

301-
const userAssociatedEditors = this.getUserAssociatedEditors(resource);
261+
const userAssociatedEditors = this.getUserAssociatedEditors(notebookUri);
302262
const notebookEditor = userAssociatedEditors.filter(association => this.notebookService.getContributedNotebookProvider(association.viewType));
303263

304264
if (userAssociatedEditors.length && !notebookEditor.length) {
@@ -309,46 +269,18 @@ export class NotebookContribution extends Disposable implements IWorkbenchContri
309269
// user might pick a notebook editor
310270

311271
const associatedEditors = distinct([
312-
...this.getUserAssociatedNotebookEditors(resource),
313-
...this.getContributedEditors(resource)
272+
...this.getUserAssociatedNotebookEditors(notebookUri),
273+
...this.getContributedEditors(notebookUri)
314274
], editor => editor.id).filter(editor => editor.priority === NotebookEditorPriority.default);
315275

316276
if (!associatedEditors.length) {
317277
// there is no notebook editor contribution which is enabled by default
318-
return;
319-
}
320-
321-
} else {
322-
const existingEditors = group.editors.filter(editor => editor.resource && isEqual(editor.resource, resource) && (editor instanceof NotebookEditorInput) && editor.viewType === id);
323-
324-
if (existingEditors.length) {
325-
// switch to this cell
326-
return { override: this.editorService.openEditor(existingEditors[0], new NotebookEditorOptions(options || {}).with({ override: false }), group) };
327-
}
328-
}
329-
330-
let info: NotebookProviderInfo | undefined;
331-
const data = CellUri.parse(resource);
332-
if (data) {
333-
const infos = this.getContributedEditors(data.notebook);
334-
335-
if (infos.length) {
336-
const info = id === undefined ? infos[0] : (infos.find(info => info.id === id) || infos[0]);
337-
// cell-uri -> open (container) notebook
338-
const name = basename(data.notebook);
339-
let input = this._resourceMapping.get(data.notebook);
340-
if (!input || input.isDisposed()) {
341-
input = NotebookEditorInput.create(this.instantiationService, data.notebook, name, info.id);
342-
this._resourceMapping.set(data.notebook, input);
343-
}
344-
345-
input.updateGroup(group.id);
346-
return { override: this.editorService.openEditor(input, new NotebookEditorOptions({ ...options, forceReload: true, cellOptions: { resource, options } }), group) };
278+
return undefined;
347279
}
348280
}
349281

350-
const infos = this.notebookService.getContributedNotebookProviders(resource);
351-
info = id === undefined ? infos[0] : infos.find(info => info.id === id);
282+
const infos = this.notebookService.getContributedNotebookProviders(notebookUri);
283+
let info = infos.find(info => !id || info.id === id);
352284

353285
if (!info && id !== undefined) {
354286
info = this.notebookService.getContributedNotebookProvider(id);
@@ -358,20 +290,19 @@ export class NotebookContribution extends Disposable implements IWorkbenchContri
358290
return undefined;
359291
}
360292

361-
const input = NotebookEditorInput.create(this.instantiationService, resource, originalInput.getName(), info.id);
362-
input.updateGroup(group.id);
363-
this._resourceMapping.set(resource, input);
364293

365294
/**
366295
* Scenario: we are reopening a file editor input which is pinned, we should open in a new editor tab.
367296
*/
368297
let index = undefined;
369-
if (group.activeEditor === originalInput && isEqual(originalInput.resource, resource)) {
298+
if (group.activeEditor === originalInput && isEqual(originalInput.resource, notebookUri)) {
370299
const originalEditorIndex = group.getIndexOfEditor(originalInput);
371300
index = group.isPinned(originalInput) ? originalEditorIndex + 1 : originalEditorIndex;
372301
}
373302

374-
return { override: this.editorService.openEditor(input, new NotebookEditorOptions(options || {}).with({ override: false, index }), group) };
303+
const notebookInput = NotebookEditorInput.create(this.instantiationService, notebookUri, originalInput.getName(), info.id);
304+
const notebookOptions = new NotebookEditorOptions({ ...options, cellOptions, override: false, index });
305+
return { override: this.editorService.openEditor(notebookInput, notebookOptions, group) };
375306
}
376307
}
377308

@@ -384,6 +315,7 @@ class CellContentProvider implements ITextModelContentProvider {
384315
@IModelService private readonly _modelService: IModelService,
385316
@IModeService private readonly _modeService: IModeService,
386317
@INotebookService private readonly _notebookService: INotebookService,
318+
@INotebookEditorModelResolverService private readonly _notebookModelResolverService: INotebookEditorModelResolverService,
387319
) {
388320
this._registration = textModelService.registerTextModelContentProvider(CellUri.scheme, this);
389321
}
@@ -407,12 +339,10 @@ class CellContentProvider implements ITextModelContentProvider {
407339
return null;
408340
}
409341

410-
const editorModel = await this._notebookService.modelManager.resolve(data.notebook, info.id);
411-
if (!editorModel) {
412-
return null;
413-
}
342+
const ref = await this._notebookModelResolverService.resolve(data.notebook, info.id);
343+
let result: ITextModel | null = null;
414344

415-
for (let cell of editorModel.notebook.cells) {
345+
for (let cell of ref.object.notebook.cells) {
416346
if (cell.uri.toString() === resource.toString()) {
417347
const bufferFactory: ITextBufferFactory = {
418348
create: (defaultEOL) => {
@@ -425,15 +355,23 @@ class CellContentProvider implements ITextModelContentProvider {
425355
}
426356
};
427357
const language = cell.cellKind === CellKind.Markdown ? this._modeService.create('markdown') : (cell.language ? this._modeService.create(cell.language) : this._modeService.createByFilepathOrFirstLine(resource, cell.textBuffer.getLineContent(1)));
428-
return this._modelService.createModel(
358+
result = this._modelService.createModel(
429359
bufferFactory,
430360
language,
431361
resource
432362
);
363+
break;
433364
}
434365
}
435366

436-
return null;
367+
if (result) {
368+
const once = result.onWillDispose(() => {
369+
once.dispose();
370+
ref.dispose();
371+
});
372+
}
373+
374+
return result;
437375
}
438376
}
439377

0 commit comments

Comments
 (0)