Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove notebook content provider save and backup functionality #160581

Merged
merged 3 commits into from
Sep 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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
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
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
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
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
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
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