Skip to content

Commit d39ba97

Browse files
jriekenjrieken
authored
Merge pull request microsoft#102760 from microsoft/joh/celldocs
Clean up and simplify cell text document world
2 parents fa2c889 + 7813e46 commit d39ba97

10 files changed

Lines changed: 304 additions & 184 deletions

File tree

‎extensions/vscode-notebook-tests/src/notebook.test.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,46 @@ suite('Notebook API tests', () => {
131131
await firstDocumentClose;
132132
});
133133

134+
test('notebook open/close, all cell-documents are ready', async function () {
135+
const resource = await createRandomFile('', undefined, 'first', '.vsctestnb');
136+
137+
const p = getEventOncePromise(vscode.notebook.onDidOpenNotebookDocument).then(notebook => {
138+
for (let cell of notebook.cells) {
139+
const doc = vscode.workspace.textDocuments.find(doc => doc.uri.toString() === cell.uri.toString());
140+
assert.ok(doc);
141+
assert.strictEqual(doc === cell.document, true);
142+
assert.strictEqual(doc?.languageId, cell.language);
143+
assert.strictEqual(doc?.isDirty, false);
144+
assert.strictEqual(doc?.isClosed, false);
145+
}
146+
});
147+
148+
await vscode.commands.executeCommand('vscode.openWith', resource, 'notebookCoreTest');
149+
await p;
150+
await vscode.commands.executeCommand('workbench.action.closeAllEditors');
151+
});
152+
153+
test('notebook open/close, notebook ready when cell-document open event is fired', async function () {
154+
const resource = await createRandomFile('', undefined, 'first', '.vsctestnb');
155+
let didHappen = false;
156+
const p = getEventOncePromise(vscode.workspace.onDidOpenTextDocument).then(doc => {
157+
if (doc.uri.scheme !== 'vscode-notebook-cell') {
158+
return;
159+
}
160+
const notebook = vscode.notebook.notebookDocuments.find(notebook => {
161+
const cell = notebook.cells.find(cell => cell.document === doc);
162+
return Boolean(cell);
163+
});
164+
assert.ok(notebook, `notebook for cell ${doc.uri} NOT found`);
165+
didHappen = true;
166+
});
167+
168+
await vscode.commands.executeCommand('vscode.openWith', resource, 'notebookCoreTest');
169+
await p;
170+
assert.strictEqual(didHappen, true);
171+
await vscode.commands.executeCommand('workbench.action.closeAllEditors');
172+
});
173+
134174
test('shared document in notebook editors', async function () {
135175
assertInitalState();
136176

‎src/vs/base/common/network.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ export namespace Schemas {
5858

5959
export const vscodeNotebook = 'vscode-notebook';
6060

61+
export const vscodeNotebookCell = 'vscode-notebook-cell';
62+
6163
export const vscodeSettings = 'vscode-settings';
6264

6365
export const webviewPanel = 'webview-panel';

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
203203

204204
async removeNotebookTextModel(uri: URI): Promise<void> {
205205
// TODO@rebornix, remove cell should use emitDelta as well to ensure document/editor events are sent together
206-
await this._proxy.$acceptDocumentAndEditorsDelta({ removedDocuments: [uri] });
206+
this._proxy.$acceptDocumentAndEditorsDelta({ removedDocuments: [uri] });
207207
let textModelDisposableStore = this._editorEventListenersMapping.get(uri.toString());
208208
textModelDisposableStore?.dispose();
209209
this._editorEventListenersMapping.delete(URI.from(uri).toString());

‎src/vs/workbench/api/common/extHost.protocol.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1633,7 +1633,7 @@ export interface ExtHostNotebookShape {
16331633
$acceptModelChanged(uriComponents: UriComponents, event: NotebookCellsChangedEvent): void;
16341634
$acceptModelSaved(uriComponents: UriComponents): void;
16351635
$acceptEditorPropertiesChanged(uriComponents: UriComponents, data: INotebookEditorPropertiesChangeData): void;
1636-
$acceptDocumentAndEditorsDelta(delta: INotebookDocumentsAndEditorsDelta): Promise<void>;
1636+
$acceptDocumentAndEditorsDelta(delta: INotebookDocumentsAndEditorsDelta): void;
16371637
$undoNotebook(viewType: string, uri: UriComponents, editId: number, isDirty: boolean): Promise<void>;
16381638
$redoNotebook(viewType: string, uri: UriComponents, editId: number, isDirty: boolean): Promise<void>;
16391639

‎src/vs/workbench/api/common/extHostDocuments.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export class ExtHostDocuments implements ExtHostDocumentsShape {
5353
}
5454

5555
public getAllDocumentData(): ExtHostDocumentData[] {
56-
return this._documentsAndEditors.allDocuments();
56+
return [...this._documentsAndEditors.allDocuments()];
5757
}
5858

5959
public getDocumentData(resource: vscode.Uri): ExtHostDocumentData | undefined {
@@ -69,8 +69,8 @@ export class ExtHostDocuments implements ExtHostDocumentsShape {
6969

7070
public getDocument(resource: vscode.Uri): vscode.TextDocument {
7171
const data = this.getDocumentData(resource);
72-
if (!data || !data.document) {
73-
throw new Error('Unable to retrieve document from URI');
72+
if (!data?.document) {
73+
throw new Error(`Unable to retrieve document from URI '${resource}'`);
7474
}
7575
return data.document;
7676
}

‎src/vs/workbench/api/common/extHostDocumentsAndEditors.ts‎

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,19 @@ import { ExtHostTextEditor } from 'vs/workbench/api/common/extHostTextEditor';
1515
import * as typeConverters from 'vs/workbench/api/common/extHostTypeConverters';
1616
import { ILogService } from 'vs/platform/log/common/log';
1717
import { ResourceMap } from 'vs/base/common/map';
18+
import { Schemas } from 'vs/base/common/network';
19+
import { Iterable } from 'vs/base/common/iterator';
20+
21+
class Reference<T> {
22+
private _count = 0;
23+
constructor(readonly value: T) { }
24+
ref() {
25+
this._count++;
26+
}
27+
unref() {
28+
return --this._count === 0;
29+
}
30+
}
1831

1932
export class ExtHostDocumentsAndEditors implements ExtHostDocumentsAndEditorsShape {
2033

@@ -23,7 +36,7 @@ export class ExtHostDocumentsAndEditors implements ExtHostDocumentsAndEditorsSha
2336
private _activeEditorId: string | null = null;
2437

2538
private readonly _editors = new Map<string, ExtHostTextEditor>();
26-
private readonly _documents = new ResourceMap<ExtHostDocumentData>();
39+
private readonly _documents = new ResourceMap<Reference<ExtHostDocumentData>>();
2740

2841
private readonly _onDidAddDocuments = new Emitter<ExtHostDocumentData[]>();
2942
private readonly _onDidRemoveDocuments = new Emitter<ExtHostDocumentData[]>();
@@ -50,29 +63,40 @@ export class ExtHostDocumentsAndEditors implements ExtHostDocumentsAndEditorsSha
5063
for (const uriComponent of delta.removedDocuments) {
5164
const uri = URI.revive(uriComponent);
5265
const data = this._documents.get(uri);
53-
this._documents.delete(uri);
54-
if (data) {
55-
removedDocuments.push(data);
66+
if (data?.unref()) {
67+
this._documents.delete(uri);
68+
removedDocuments.push(data.value);
5669
}
5770
}
5871
}
5972

6073
if (delta.addedDocuments) {
6174
for (const data of delta.addedDocuments) {
6275
const resource = URI.revive(data.uri);
63-
assert.ok(!this._documents.has(resource), `document '${resource} already exists!'`);
64-
65-
const documentData = new ExtHostDocumentData(
66-
this._extHostRpc.getProxy(MainContext.MainThreadDocuments),
67-
resource,
68-
data.lines,
69-
data.EOL,
70-
data.modeId,
71-
data.versionId,
72-
data.isDirty
73-
);
74-
this._documents.set(resource, documentData);
75-
addedDocuments.push(documentData);
76+
let ref = this._documents.get(resource);
77+
78+
// double check -> only notebook cell documents should be
79+
// referenced/opened more than once...
80+
if (ref) {
81+
if (resource.scheme !== Schemas.vscodeNotebookCell) {
82+
throw new Error(`document '${resource} already exists!'`);
83+
}
84+
}
85+
if (!ref) {
86+
ref = new Reference(new ExtHostDocumentData(
87+
this._extHostRpc.getProxy(MainContext.MainThreadDocuments),
88+
resource,
89+
data.lines,
90+
data.EOL,
91+
data.modeId,
92+
data.versionId,
93+
data.isDirty
94+
));
95+
this._documents.set(resource, ref);
96+
addedDocuments.push(ref.value);
97+
}
98+
99+
ref.ref();
76100
}
77101
}
78102

@@ -92,7 +116,7 @@ export class ExtHostDocumentsAndEditors implements ExtHostDocumentsAndEditorsSha
92116
assert.ok(this._documents.has(resource), `document '${resource}' does not exist`);
93117
assert.ok(!this._editors.has(data.id), `editor '${data.id}' already exists!`);
94118

95-
const documentData = this._documents.get(resource)!;
119+
const documentData = this._documents.get(resource)!.value;
96120
const editor = new ExtHostTextEditor(
97121
data.id,
98122
this._extHostRpc.getProxy(MainContext.MainThreadTextEditors),
@@ -132,11 +156,11 @@ export class ExtHostDocumentsAndEditors implements ExtHostDocumentsAndEditorsSha
132156
}
133157

134158
getDocument(uri: URI): ExtHostDocumentData | undefined {
135-
return this._documents.get(uri);
159+
return this._documents.get(uri)?.value;
136160
}
137161

138-
allDocuments(): ExtHostDocumentData[] {
139-
return [...this._documents.values()];
162+
allDocuments(): Iterable<ExtHostDocumentData> {
163+
return Iterable.map(this._documents.values(), ref => ref.value);
140164
}
141165

142166
getEditor(id: string): ExtHostTextEditor | undefined {

0 commit comments

Comments
 (0)