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 2 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
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import * as assert from 'assert';
import { Barrier } from 'vs/base/common/async';
import { DisposableStore } from 'vs/base/common/lifecycle';
import { URI, UriComponents } from 'vs/base/common/uri';
import { generateUuid } from 'vs/base/common/uuid';
import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
import { NullLogService } from 'vs/platform/log/common/log';
import { ICellExecuteUpdateDto, ICellExecutionCompleteDto, INotebookKernelDto2, MainContext, MainThreadCommandsShape, MainThreadNotebookDocumentsShape, MainThreadNotebookKernelsShape, MainThreadNotebookShape } from 'vs/workbench/api/common/extHost.protocol';
Expand All @@ -19,7 +18,6 @@ import { ExtHostNotebookController } from 'vs/workbench/api/common/extHostNotebo
import { ExtHostNotebookDocument } from 'vs/workbench/api/common/extHostNotebookDocument';
import { ExtHostNotebookDocuments } from 'vs/workbench/api/common/extHostNotebookDocuments';
import { ExtHostNotebookKernels } from 'vs/workbench/api/common/extHostNotebookKernels';
import { IExtensionStoragePaths } from 'vs/workbench/api/common/extHostStoragePaths';
import { NotebookCellOutput, NotebookCellOutputItem } from 'vs/workbench/api/common/extHostTypes';
import { CellKind, CellUri, NotebookCellsChangeType } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { CellExecutionUpdateType } from 'vs/workbench/contrib/notebook/common/notebookExecutionService';
Expand Down Expand Up @@ -90,13 +88,8 @@ suite('NotebookKernel', 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() });
}
};
extHostCommands = new ExtHostCommands(rpcProtocol, new NullLogService());
extHostNotebooks = new ExtHostNotebookController(rpcProtocol, extHostCommands, extHostDocumentsAndEditors, extHostDocuments, extHostStoragePaths);
extHostNotebooks = new ExtHostNotebookController(rpcProtocol, extHostCommands, extHostDocumentsAndEditors, extHostDocuments);

extHostNotebookDocuments = new ExtHostNotebookDocuments(extHostNotebooks);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,6 @@ export class InteractiveDocumentContribution extends Disposable implements IWork
transientOptions: contentOptions
};
},
save: async (uri: URI) => {
// trigger backup always
return false;
},
saveAs: async (uri: URI, target: URI, token: CancellationToken) => {
// return this._proxy.$saveNotebookAs(viewType, uri, target, token);
return false;
},
backup: async (uri: URI, token: CancellationToken) => {
const doc = notebookService.listNotebookDocuments().find(document => document.uri.toString() === uri.toString());
if (doc) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ export class ComplexNotebookEditorModel extends EditorModel implements INotebook
if (!this.isResolved()) {
return;
}
const success = await this._contentProvider.save(this.notebook.uri, CancellationToken.None);
const success = false;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not familiar enough with this code to do a better fix

The interactive window always returns false for these methods

this._logService.debug(`[notebook editor model] save(${versionId}) - document saved saved, start updating file stats`, this.resource.toString(true), success);
this._lastResolvedFileStat = await this._resolveStats(this.resource);
if (success) {
Expand Down Expand Up @@ -407,7 +407,7 @@ export class ComplexNotebookEditorModel extends EditorModel implements INotebook
return undefined;
}

const success = await this._contentProvider.saveAs(this.notebook.uri, targetResource, CancellationToken.None);
const success = false;
this._logService.debug(`[notebook editor model] saveAs - document saved, start updating file stats`, this.resource.toString(true), success);
this._lastResolvedFileStat = await this._resolveStats(this.resource);
if (!success) {
Expand Down
Loading