Skip to content

Commit

Permalink
Remove notebook content provider save and backup functionality
Browse files Browse the repository at this point in the history 
For microsoft#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 committed Sep 9, 2022
1 parent 0c1f65f commit b3b4fc9
Show file tree
Hide file tree
Showing 14 changed files with 12 additions and 171 deletions.
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
9 changes: 1 addition & 8 deletions src/vs/workbench/api/test/browser/extHostNotebookKernel.test.ts
View file
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
8 changes: 0 additions & 8 deletions src/vs/workbench/contrib/interactive/browser/interactive.contribution.ts
View file
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
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts
View file
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;
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
2 changes: 0 additions & 2 deletions src/vs/workbench/contrib/notebook/common/notebookService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ export interface INotebookContentProvider {
options: TransientOptions;

open(uri: URI, backupId: string | VSBuffer | undefined, untitledDocumentData: VSBuffer | undefined, token: CancellationToken): Promise<{ data: NotebookData; transientOptions: TransientOptions }>;
save(uri: URI, token: CancellationToken): Promise<boolean>;
saveAs(uri: URI, target: URI, token: CancellationToken): Promise<boolean>;
backup(uri: URI, token: CancellationToken): Promise<string | VSBuffer>;
}

Expand Down
Loading

0 comments on commit b3b4fc9

Please sign in to comment.