Skip to content

Commit

Permalink
Remove notebook content provider save and backup functionality (#160581)
Browse files Browse the repository at this point in the history 
For #147248

When stepping through the liveshare code, we figured out they do not appear to be using `save`, `saveAs`, or `backup` (they return empty results for these)

This PR removes this part of the proposal so we can track just the parts of the api that they are using
  • Loading branch information
@mjbvz
mjbvz authored Sep 12, 2022
1 parent 8717b43 commit 2514759
Show file tree
Hide file tree
Showing 16 changed files with 14 additions and 242 deletions.
15 changes: 2 additions & 13 deletions extensions/vscode-api-tests/src/singlefolder-tests/notebook.api.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,18 +115,6 @@ const apiTestContentProvider: vscode.NotebookContentProvider = {
};
return dto;
},
saveNotebook: async (_document: vscode.NotebookDocument, _cancellation: vscode.CancellationToken) => {
return;
},
saveNotebookAs: async (_targetResource: vscode.Uri, _document: vscode.NotebookDocument, _cancellation: vscode.CancellationToken) => {
return;
},
backupNotebook: async (_document: vscode.NotebookDocument, _context: vscode.NotebookDocumentBackupContext, _cancellation: vscode.CancellationToken) => {
return {
id: '1',
delete: () => { }
};
}
};

(vscode.env.uiKind === vscode.UIKind.Web ? suite.skip : suite)('Notebook API tests', function () {
Expand Down Expand Up @@ -244,7 +232,8 @@ const apiTestContentProvider: vscode.NotebookContentProvider = {
await closeAllEditors();
});

test('#115855 onDidSaveNotebookDocument', async function () {
// TODO: Skipped due to notebook content provider removal
test.skip('#115855 onDidSaveNotebookDocument', async function () {
const resource = await createRandomNotebookFile();
const notebook = await vscode.workspace.openNotebookDocument(resource);

Expand Down
58 changes: 0 additions & 58 deletions extensions/vscode-api-tests/src/singlefolder-tests/notebook.document.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,6 @@ suite('Notebook Document', function () {
[new vscode.NotebookCellData(vscode.NotebookCellKind.Code, uri.toString(), 'javascript')],
);
}
async saveNotebook(_document: vscode.NotebookDocument, _cancellation: vscode.CancellationToken) {
//
}
async saveNotebookAs(_targetResource: vscode.Uri, _document: vscode.NotebookDocument, _cancellation: vscode.CancellationToken) {
//
}
async backupNotebook(_document: vscode.NotebookDocument, _context: vscode.NotebookDocumentBackupContext, _cancellation: vscode.CancellationToken) {
return { id: '', delete() { } };
}
};

const disposables: vscode.Disposable[] = [];
Expand Down Expand Up @@ -329,40 +320,6 @@ suite('Notebook Document', function () {
assert.ok(document.metadata.custom.extraNotebookMetadata, `Test metadata not found`);
});

test('document save API', async function () {
const uri = await utils.createRandomFile(undefined, undefined, '.nbdtest');
const notebook = await vscode.workspace.openNotebookDocument(uri);

assert.strictEqual(notebook.uri.toString(), uri.toString());
assert.strictEqual(notebook.isDirty, false);
assert.strictEqual(notebook.isUntitled, false);
assert.strictEqual(notebook.cellCount, 1);
assert.strictEqual(notebook.notebookType, 'notebook.nbdtest');

const edit = new vscode.WorkspaceEdit();
edit.set(notebook.uri, [vscode.NotebookEdit.replaceCells(new vscode.NotebookRange(0, 0), [{
kind: vscode.NotebookCellKind.Markup,
languageId: 'markdown',
metadata: undefined,
outputs: [],
value: 'new_markdown'
}, {
kind: vscode.NotebookCellKind.Code,
languageId: 'fooLang',
metadata: undefined,
outputs: [],
value: 'new_code'
}])]);

const success = await vscode.workspace.applyEdit(edit);
assert.strictEqual(success, true);
assert.strictEqual(notebook.isDirty, true);

await notebook.save();
assert.strictEqual(notebook.isDirty, false);
});


test('setTextDocumentLanguage for notebook cells', async function () {

const uri = await utils.createRandomFile(undefined, undefined, '.nbdtest');
Expand Down Expand Up @@ -395,21 +352,6 @@ suite('Notebook Document', function () {
assert.strictEqual(cellDoc.languageId, 'css');
});

test('dirty state - complex', async function () {
const resource = await utils.createRandomFile(undefined, undefined, '.nbdtest');
const document = await vscode.workspace.openNotebookDocument(resource);
assert.strictEqual(document.isDirty, false);

const edit = new vscode.WorkspaceEdit();
edit.set(document.uri, [vscode.NotebookEdit.replaceCells(new vscode.NotebookRange(0, document.cellCount), [])]);
assert.ok(await vscode.workspace.applyEdit(edit));

assert.strictEqual(document.isDirty, true);

await document.save();
assert.strictEqual(document.isDirty, false);
});

test('dirty state - serializer', async function () {
const resource = await utils.createRandomFile(undefined, undefined, '.nbdserializer');
const document = await vscode.workspace.openNotebookDocument(resource);
Expand Down
12 changes: 0 additions & 12 deletions extensions/vscode-api-tests/src/singlefolder-tests/notebook.kernel.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,18 +124,6 @@ const apiTestContentProvider: vscode.NotebookContentProvider = {
]
};
return dto;
},
saveNotebook: async (_document: vscode.NotebookDocument, _cancellation: vscode.CancellationToken) => {
return;
},
saveNotebookAs: async (_targetResource: vscode.Uri, _document: vscode.NotebookDocument, _cancellation: vscode.CancellationToken) => {
return;
},
backupNotebook: async (_document: vscode.NotebookDocument, _context: vscode.NotebookDocumentBackupContext, _cancellation: vscode.CancellationToken) => {
return {
id: '1',
delete: () => { }
};
}
};

Expand Down
12 changes: 0 additions & 12 deletions extensions/vscode-notebook-tests/src/extension.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,6 @@ export function activate(context: vscode.ExtensionContext): any {
};

return dto;
},
saveNotebook: async (_document: vscode.NotebookDocument, _cancellation: vscode.CancellationToken) => {
return;
},
saveNotebookAs: async (_targetResource: vscode.Uri, _document: vscode.NotebookDocument, _cancellation: vscode.CancellationToken) => {
return;
},
backupNotebook: async (_document: vscode.NotebookDocument, _context: vscode.NotebookDocumentBackupContext, _cancellation: vscode.CancellationToken) => {
return {
id: '1',
delete: () => { }
};
}
}));

Expand Down
26 changes: 2 additions & 24 deletions src/vs/workbench/api/browser/mainThreadNotebook.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { URI } from 'vs/base/common/uri';
import { NotebookDto } from 'vs/workbench/api/browser/mainThreadNotebookDto';
import { extHostNamedCustomer, IExtHostContext } from 'vs/workbench/services/extensions/common/extHostCustomers';
import { INotebookCellStatusBarService } from 'vs/workbench/contrib/notebook/common/notebookCellStatusBarService';
import { INotebookCellStatusBarItemProvider, INotebookContributionData, NotebookData as NotebookData, NotebookExtensionDescription, TransientCellMetadata, TransientDocumentMetadata, TransientOptions } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { INotebookCellStatusBarItemProvider, INotebookContributionData, NotebookData as NotebookData, NotebookExtensionDescription, TransientOptions } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { INotebookContentProvider, INotebookService, SimpleNotebookProviderInfo } from 'vs/workbench/contrib/notebook/common/notebookService';
import { SerializableObjectWithBuffers } from 'vs/workbench/services/extensions/common/proxyIdentifier';
import { ExtHostContext, ExtHostNotebookShape, MainContext, MainThreadNotebookShape } from '../common/extHost.protocol';
Expand Down Expand Up @@ -67,15 +67,7 @@ export class MainThreadNotebooks implements MainThreadNotebookShape {
transientOptions: contentOptions
};
},
save: async (uri: URI, token: CancellationToken) => {
return this._proxy.$saveNotebook(viewType, uri, token);
},
saveAs: async (uri: URI, target: URI, token: CancellationToken) => {
return this._proxy.$saveNotebookAs(viewType, uri, target, token);
},
backup: async (uri: URI, token: CancellationToken) => {
return this._proxy.$backupNotebook(viewType, uri, token);
}
backup: async (uri: URI, token: CancellationToken) => ''
};

const disposable = new DisposableStore();
Expand All @@ -86,19 +78,6 @@ export class MainThreadNotebooks implements MainThreadNotebookShape {
this._notebookProviders.set(viewType, { controller, disposable });
}

async $updateNotebookProviderOptions(viewType: string, options?: { transientOutputs: boolean; transientCellMetadata: TransientCellMetadata; transientDocumentMetadata: TransientDocumentMetadata }): Promise<void> {
const provider = this._notebookProviders.get(viewType);

if (provider && options) {
provider.controller.options = options;
this._notebookService.listNotebookDocuments().forEach(document => {
if (document.viewType === viewType) {
document.transientOptions = provider.controller.options;
}
});
}
}

async $unregisterNotebookProvider(viewType: string): Promise<void> {
const entry = this._notebookProviders.get(viewType);
if (entry) {
Expand All @@ -107,7 +86,6 @@ export class MainThreadNotebooks implements MainThreadNotebookShape {
}
}


$registerNotebookSerializer(handle: number, extension: NotebookExtensionDescription, viewType: string, options: TransientOptions, data: INotebookContributionData | undefined): void {
const registration = this._notebookService.registerNotebookSerializer(viewType, extension, {
options,
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/api/common/extHost.api.impl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
const extHostDocuments = rpcProtocol.set(ExtHostContext.ExtHostDocuments, new ExtHostDocuments(rpcProtocol, extHostDocumentsAndEditors));
const extHostDocumentContentProviders = rpcProtocol.set(ExtHostContext.ExtHostDocumentContentProviders, new ExtHostDocumentContentProvider(rpcProtocol, extHostDocumentsAndEditors, extHostLogService));
const extHostDocumentSaveParticipant = rpcProtocol.set(ExtHostContext.ExtHostDocumentSaveParticipant, new ExtHostDocumentSaveParticipant(extHostLogService, extHostDocuments, rpcProtocol.getProxy(MainContext.MainThreadBulkEdits)));
const extHostNotebook = rpcProtocol.set(ExtHostContext.ExtHostNotebook, new ExtHostNotebookController(rpcProtocol, extHostCommands, extHostDocumentsAndEditors, extHostDocuments, extensionStoragePaths));
const extHostNotebook = rpcProtocol.set(ExtHostContext.ExtHostNotebook, new ExtHostNotebookController(rpcProtocol, extHostCommands, extHostDocumentsAndEditors, extHostDocuments));
const extHostNotebookDocuments = rpcProtocol.set(ExtHostContext.ExtHostNotebookDocuments, new ExtHostNotebookDocuments(extHostNotebook));
const extHostNotebookEditors = rpcProtocol.set(ExtHostContext.ExtHostNotebookEditors, new ExtHostNotebookEditors(extHostLogService, extHostNotebook));
const extHostNotebookKernels = rpcProtocol.set(ExtHostContext.ExtHostNotebookKernels, new ExtHostNotebookKernels(rpcProtocol, initData, extHostNotebook, extHostCommands, extHostLogService));
Expand Down
4 changes: 0 additions & 4 deletions src/vs/workbench/api/common/extHost.protocol.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,6 @@ export interface INotebookCellStatusBarListDto {

export interface MainThreadNotebookShape extends IDisposable {
$registerNotebookProvider(extension: notebookCommon.NotebookExtensionDescription, viewType: string, options: notebookCommon.TransientOptions, registration: notebookCommon.INotebookContributionData | undefined): Promise<void>;
$updateNotebookProviderOptions(viewType: string, options?: { transientOutputs: boolean; transientCellMetadata: notebookCommon.TransientCellMetadata; transientDocumentMetadata: notebookCommon.TransientDocumentMetadata }): Promise<void>;
$unregisterNotebookProvider(viewType: string): Promise<void>;

$registerNotebookSerializer(handle: number, extension: notebookCommon.NotebookExtensionDescription, viewType: string, options: notebookCommon.TransientOptions, registration: notebookCommon.INotebookContributionData | undefined): void;
Expand Down Expand Up @@ -2056,9 +2055,6 @@ export interface ExtHostNotebookShape extends ExtHostNotebookDocumentsAndEditors
$releaseNotebookCellStatusBarItems(id: number): void;

$openNotebook(viewType: string, uri: UriComponents, backupId: string | undefined, untitledDocumentData: VSBuffer | undefined, token: CancellationToken): Promise<SerializableObjectWithBuffers<NotebookDataDto>>;
$saveNotebook(viewType: string, uri: UriComponents, token: CancellationToken): Promise<boolean>;
$saveNotebookAs(viewType: string, uri: UriComponents, target: UriComponents, token: CancellationToken): Promise<boolean>;
$backupNotebook(viewType: string, uri: UriComponents, cancellation: CancellationToken): Promise<string>;

$dataToNotebook(handle: number, data: VSBuffer, token: CancellationToken): Promise<SerializableObjectWithBuffers<NotebookDataDto>>;
$notebookToData(handle: number, data: SerializableObjectWithBuffers<NotebookDataDto>, token: CancellationToken): Promise<VSBuffer>;
Expand Down
42 changes: 0 additions & 42 deletions src/vs/workbench/api/common/extHostNotebook.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { VSBuffer } from 'vs/base/common/buffer';
import { CancellationToken } from 'vs/base/common/cancellation';
import { Emitter, Event } from 'vs/base/common/event';
import { IRelativePattern } from 'vs/base/common/glob';
import { hash } from 'vs/base/common/hash';
import { DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { ResourceMap } from 'vs/base/common/map';
import { MarshalledId } from 'vs/base/common/marshallingIds';
Expand All @@ -20,7 +19,6 @@ import { ExtHostNotebookShape, IMainContext, IModelAddedData, INotebookCellStatu
import { ApiCommand, ApiCommandArgument, ApiCommandResult, CommandsConverter, ExtHostCommands } from 'vs/workbench/api/common/extHostCommands';
import { ExtHostDocuments } from 'vs/workbench/api/common/extHostDocuments';
import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/common/extHostDocumentsAndEditors';
import { IExtensionStoragePaths } from 'vs/workbench/api/common/extHostStoragePaths';
import * as typeConverters from 'vs/workbench/api/common/extHostTypeConverters';
import * as extHostTypes from 'vs/workbench/api/common/extHostTypes';
import { INotebookExclusiveDocumentFilter, INotebookContributionData } from 'vs/workbench/contrib/notebook/common/notebookCommon';
Expand Down Expand Up @@ -75,7 +73,6 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {
commands: ExtHostCommands,
private _textDocumentsAndEditors: ExtHostDocumentsAndEditors,
private _textDocuments: ExtHostDocuments,
private readonly _extensionStoragePaths: IExtensionStoragePaths,
) {
this._notebookProxy = mainContext.getProxy(MainContext.MainThreadNotebook);
this._notebookDocumentsProxy = mainContext.getProxy(MainContext.MainThreadNotebookDocuments);
Expand Down Expand Up @@ -157,14 +154,6 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {

this._notebookContentProviders.set(viewType, { extension, provider });

let listener: IDisposable | undefined;
if (provider.onDidChangeNotebookContentOptions) {
listener = provider.onDidChangeNotebookContentOptions(() => {
const internalOptions = typeConverters.NotebookDocumentContentOptions.from(provider.options);
this._notebookProxy.$updateNotebookProviderOptions(viewType, internalOptions);
});
}

this._notebookProxy.$registerNotebookProvider(
{ id: extension.identifier, location: extension.extensionLocation },
viewType,
Expand All @@ -173,7 +162,6 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {
);

return new extHostTypes.Disposable(() => {
listener?.dispose();
this._notebookContentProviders.delete(viewType);
this._notebookProxy.$unregisterNotebookProvider(viewType);
});
Expand Down Expand Up @@ -356,36 +344,6 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {
});
}

async $saveNotebook(viewType: string, uri: UriComponents, token: CancellationToken): Promise<boolean> {
const document = this.getNotebookDocument(URI.revive(uri));
const { provider } = this._getProviderData(viewType);
await provider.saveNotebook(document.apiNotebook, token);
return true;
}

async $saveNotebookAs(viewType: string, uri: UriComponents, target: UriComponents, token: CancellationToken): Promise<boolean> {
const document = this.getNotebookDocument(URI.revive(uri));
const { provider } = this._getProviderData(viewType);
await provider.saveNotebookAs(URI.revive(target), document.apiNotebook, token);
return true;
}

private _backupIdPool: number = 0;

async $backupNotebook(viewType: string, uri: UriComponents, cancellation: CancellationToken): Promise<string> {
const document = this.getNotebookDocument(URI.revive(uri));
const provider = this._getProviderData(viewType);

const storagePath = this._extensionStoragePaths.workspaceValue(provider.extension) ?? this._extensionStoragePaths.globalValue(provider.extension);
const fileName = String(hash([document.uri.toString(), this._backupIdPool++]));
const backupUri = URI.joinPath(storagePath, fileName);

const backup = await provider.provider.backupNotebook(document.apiNotebook, { destination: backupUri }, cancellation);
document.updateBackup(backup);
return backup.id;
}


private _createExtHostEditor(document: ExtHostNotebookDocument, editorId: string, data: INotebookEditorAddData) {

if (this._editors.has(editorId)) {
Expand Down
11 changes: 0 additions & 11 deletions src/vs/workbench/api/common/extHostNotebookDocument.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ export class ExtHostNotebookDocument {
private _metadata: Record<string, any>;
private _versionId: number = 0;
private _isDirty: boolean = false;
private _backup?: vscode.NotebookDocumentBackup;
private _disposed: boolean = false;

constructor(
Expand Down Expand Up @@ -190,16 +189,6 @@ export class ExtHostNotebookDocument {
return this._notebook;
}

updateBackup(backup: vscode.NotebookDocumentBackup): void {
this._backup?.delete();
this._backup = backup;
}

disposeBackup(): void {
this._backup?.delete();
this._backup = undefined;
}

acceptDocumentPropertiesChanged(data: extHostProtocol.INotebookDocumentPropertiesChangeData) {
if (data.metadata) {
this._metadata = Object.freeze({ ...this._metadata, ...data.metadata });
Expand Down
9 changes: 1 addition & 8 deletions src/vs/workbench/api/test/browser/extHostNotebook.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ import { ExtHostDocuments } from 'vs/workbench/api/common/extHostDocuments';
import { ExtHostCommands } from 'vs/workbench/api/common/extHostCommands';
import { nullExtensionDescription } from 'vs/workbench/services/extensions/common/extensions';
import { isEqual } from 'vs/base/common/resources';
import { IExtensionStoragePaths } from 'vs/workbench/api/common/extHostStoragePaths';
import { generateUuid } from 'vs/base/common/uuid';
import { Event } from 'vs/base/common/event';
import { ExtHostNotebookDocuments } from 'vs/workbench/api/common/extHostNotebookDocuments';
import { SerializableObjectWithBuffers } from 'vs/workbench/services/extensions/common/proxyIdentifier';
Expand Down Expand Up @@ -54,12 +52,7 @@ suite('NotebookCell#Document', function () {
});
extHostDocumentsAndEditors = new ExtHostDocumentsAndEditors(rpcProtocol, new NullLogService());
extHostDocuments = new ExtHostDocuments(rpcProtocol, extHostDocumentsAndEditors);
const extHostStoragePaths = new class extends mock<IExtensionStoragePaths>() {
override workspaceValue() {
return URI.from({ scheme: 'test', path: generateUuid() });
}
};
extHostNotebooks = new ExtHostNotebookController(rpcProtocol, new ExtHostCommands(rpcProtocol, new NullLogService()), extHostDocumentsAndEditors, extHostDocuments, extHostStoragePaths);
extHostNotebooks = new ExtHostNotebookController(rpcProtocol, new ExtHostCommands(rpcProtocol, new NullLogService()), extHostDocumentsAndEditors, extHostDocuments);
extHostNotebookDocuments = new ExtHostNotebookDocuments(extHostNotebooks);

const reg = extHostNotebooks.registerNotebookContentProvider(nullExtensionDescription, 'test', new class extends mock<vscode.NotebookContentProvider>() {
Expand Down
Loading

0 comments on commit 2514759

Please sign in to comment.