Skip to content

Commit ce19502

Browse files
jriekenjrieken
committed
chore - tweak onDidAddNotebookDocument, onDidRemoveNotebookDocument event, use ResourceMap and fix confusion between models and editors
1 parent fc4163e commit ce19502

3 files changed

Lines changed: 36 additions & 41 deletions

File tree

‎src/vs/workbench/api/browser/mainThreadNotebook.ts‎

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as DOM from 'vs/base/browser/dom';
77
import { CancellationToken } from 'vs/base/common/cancellation';
88
import { Emitter } from 'vs/base/common/event';
99
import { combinedDisposable, Disposable, DisposableStore, IDisposable } from 'vs/base/common/lifecycle';
10+
import { ResourceMap } from 'vs/base/common/map';
1011
import { URI, UriComponents } from 'vs/base/common/uri';
1112
import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility';
1213
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
@@ -134,7 +135,7 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
134135
private _toDisposeOnEditorRemove = new Map<string, IDisposable>();
135136
private _currentState?: DocumentAndEditorState;
136137
private _editorEventListenersMapping: Map<string, DisposableStore> = new Map();
137-
private _documentEventListenersMapping: Map<string, DisposableStore> = new Map();
138+
private _documentEventListenersMapping: ResourceMap<DisposableStore> = new ResourceMap();
138139
private readonly _cellStatusBarEntries: Map<number, IDisposable> = new Map();
139140

140141
constructor(
@@ -162,11 +163,11 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
162163
}
163164

164165
async removeNotebookTextModel(uri: URI): Promise<void> {
166+
// TODO@rebornix remove this? obsolete?
165167
// TODO@rebornix, remove cell should use emitDelta as well to ensure document/editor events are sent together
166168
this._proxy.$acceptDocumentAndEditorsDelta({ removedDocuments: [uri] });
167-
let textModelDisposableStore = this._documentEventListenersMapping.get(uri.toString());
168-
textModelDisposableStore?.dispose();
169-
this._documentEventListenersMapping.delete(URI.from(uri).toString());
169+
this._documentEventListenersMapping.get(uri)?.dispose();
170+
this._documentEventListenersMapping.delete(uri);
170171
}
171172

172173
private _isDeltaEmpty(delta: INotebookDocumentsAndEditorsDelta) {
@@ -255,40 +256,32 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
255256
notebookEditorAddedHandler(editor);
256257
});
257258

258-
const notebookDocumentAddedHandler = (doc: URI) => {
259-
if (!this._editorEventListenersMapping.has(doc.toString())) {
260-
const disposableStore = new DisposableStore();
261-
const textModel = this._notebookService.getNotebookTextModel(doc);
262-
disposableStore.add(textModel!.onDidModelChangeProxy(e => {
263-
this._proxy.$acceptModelChanged(textModel!.uri, e, textModel!.isDirty);
264-
this._proxy.$acceptDocumentPropertiesChanged(doc, { selections: { selections: textModel!.selections }, metadata: null });
265-
}));
266-
disposableStore.add(textModel!.onDidSelectionChange(e => {
267-
const selectionsChange = e ? { selections: e } : null;
268-
this._proxy.$acceptDocumentPropertiesChanged(doc, { selections: selectionsChange, metadata: null });
269-
}));
270-
271-
this._editorEventListenersMapping.set(textModel!.uri.toString(), disposableStore);
259+
const notebookDocumentAddedHandler = (textModel: NotebookTextModel) => {
260+
if (this._documentEventListenersMapping.has(textModel.uri)) {
261+
return;
272262
}
263+
const disposableStore = new DisposableStore();
264+
disposableStore.add(textModel.onDidModelChangeProxy(e => {
265+
this._proxy.$acceptModelChanged(textModel!.uri, e, textModel!.isDirty);
266+
this._proxy.$acceptDocumentPropertiesChanged(textModel.uri, { selections: { selections: textModel!.selections }, metadata: null });
267+
}));
268+
disposableStore.add(textModel.onDidSelectionChange(e => {
269+
const selectionsChange = e ? { selections: e } : null;
270+
this._proxy.$acceptDocumentPropertiesChanged(textModel.uri, { selections: selectionsChange, metadata: null });
271+
}));
272+
273+
this._documentEventListenersMapping.set(textModel.uri, disposableStore);
273274
};
274275

275-
this._register(this._notebookService.onNotebookDocumentAdd((documents) => {
276-
documents.forEach(doc => {
277-
notebookDocumentAddedHandler(doc);
278-
});
276+
this._notebookService.listNotebookDocuments().forEach(notebookDocumentAddedHandler);
277+
this._register(this._notebookService.onDidAddNotebookDocument(document => {
278+
notebookDocumentAddedHandler(document);
279279
this._updateState();
280280
}));
281281

282-
this._notebookService.listNotebookDocuments().forEach((doc) => {
283-
notebookDocumentAddedHandler(doc.uri);
284-
});
285-
286-
this._register(this._notebookService.onNotebookDocumentRemove((documents) => {
287-
documents.forEach(doc => {
288-
this._documentEventListenersMapping.get(doc.toString())?.dispose();
289-
this._documentEventListenersMapping.delete(doc.toString());
290-
});
291-
282+
this._register(this._notebookService.onDidRemoveNotebookDocument(uri => {
283+
this._documentEventListenersMapping.get(uri)?.dispose();
284+
this._documentEventListenersMapping.delete(uri);
292285
this._updateState();
293286
}));
294287

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -239,10 +239,12 @@ export class NotebookService extends Disposable implements INotebookService, ICu
239239
public readonly onNotebookEditorAdd: Event<INotebookEditor> = this._onNotebookEditorAdd.event;
240240
private readonly _onNotebookEditorsRemove: Emitter<INotebookEditor[]> = this._register(new Emitter<INotebookEditor[]>());
241241
public readonly onNotebookEditorsRemove: Event<INotebookEditor[]> = this._onNotebookEditorsRemove.event;
242-
private readonly _onNotebookDocumentAdd: Emitter<URI[]> = this._register(new Emitter<URI[]>());
243-
public readonly onNotebookDocumentAdd: Event<URI[]> = this._onNotebookDocumentAdd.event;
244-
private readonly _onNotebookDocumentRemove: Emitter<URI[]> = this._register(new Emitter<URI[]>());
245-
public readonly onNotebookDocumentRemove: Event<URI[]> = this._onNotebookDocumentRemove.event;
242+
243+
private readonly _onDidAddNotebookDocument = this._register(new Emitter<NotebookTextModel>());
244+
private readonly _onDidRemoveNotebookDocument = this._register(new Emitter<URI>());
245+
readonly onDidAddNotebookDocument = this._onDidAddNotebookDocument.event;
246+
readonly onDidRemoveNotebookDocument = this._onDidRemoveNotebookDocument.event;
247+
246248
private readonly _onNotebookDocumentSaved: Emitter<URI> = this._register(new Emitter<URI>());
247249
public readonly onNotebookDocumentSaved: Event<URI> = this._onNotebookDocumentSaved.event;
248250
private readonly _notebookEditors = new Map<string, INotebookEditor>();
@@ -631,7 +633,7 @@ export class NotebookService extends Disposable implements INotebookService, ICu
631633
);
632634

633635
this._models.set(uri, modelData);
634-
this._onNotebookDocumentAdd.fire([notebookModel.uri]);
636+
this._onDidAddNotebookDocument.fire(notebookModel);
635637
// after the document is added to the store and sent to ext host, we transform the ouputs
636638
await this.transformTextModelOutputs(notebookModel);
637639

@@ -928,8 +930,8 @@ export class NotebookService extends Disposable implements INotebookService, ICu
928930

929931

930932
this._onNotebookEditorsRemove.fire(willRemovedEditors.map(e => e));
931-
this._onNotebookDocumentRemove.fire([modelData.model.uri]);
932-
modelData?.dispose();
933+
this._onDidRemoveNotebookDocument.fire(modelData.model.uri);
934+
modelData.dispose();
933935
}
934936
}
935937
}

‎src/vs/workbench/contrib/notebook/common/notebookService.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ export interface INotebookService {
4141
onDidChangeVisibleEditors: Event<string[]>;
4242
onNotebookEditorAdd: Event<IEditor>;
4343
onNotebookEditorsRemove: Event<IEditor[]>;
44-
onNotebookDocumentRemove: Event<URI[]>;
45-
onNotebookDocumentAdd: Event<URI[]>;
44+
onDidRemoveNotebookDocument: Event<URI>;
45+
onDidAddNotebookDocument: Event<NotebookTextModel>;
4646
onNotebookDocumentSaved: Event<URI>;
4747
onDidChangeKernels: Event<URI | undefined>;
4848
onDidChangeNotebookActiveKernel: Event<{ uri: URI, providerHandle: number | undefined, kernelId: string | undefined }>;

0 commit comments

Comments
 (0)