From 0489ddff29e4eac89f69b61b2994502a0bf1a1ed Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 17 Sep 2020 16:52:27 -0700 Subject: [PATCH 1/3] Disable split views of custom editors (#13985) --- src/client/datascience/notebookStorage/nativeEditorProvider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/datascience/notebookStorage/nativeEditorProvider.ts b/src/client/datascience/notebookStorage/nativeEditorProvider.ts index 5d166c119789..ed9967675ab8 100644 --- a/src/client/datascience/notebookStorage/nativeEditorProvider.ts +++ b/src/client/datascience/notebookStorage/nativeEditorProvider.ts @@ -119,7 +119,7 @@ export class NativeEditorProvider implements INotebookEditorProvider, CustomEdit enableFindWidget: true, retainContextWhenHidden: true }, - supportsMultipleEditorsPerDocument: true + supportsMultipleEditorsPerDocument: false }); } From f881e0318906c3725a4643d8176688c5e6c1c263 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 18 Sep 2020 09:53:15 -0700 Subject: [PATCH 2/3] Fix backup storage by looking at the options correctly (#13983) * Fix backup storage by looking at the options correctly * Fix backup by being more explicit * Only linux tests are failing. Hopefully fix them --- news/2 Fixes/13981.md | 1 + src/client/datascience/export/exportUtil.ts | 4 +- .../nativeEditorProviderOld.ts | 4 +- .../interactive-ipynb/trustCommandHandler.ts | 2 +- .../interactive-window/interactiveWindow.ts | 2 +- .../datascience/notebook/contentProvider.ts | 15 ++-- .../notebook/notebookEditorProvider.ts | 2 +- .../notebookStorage/nativeEditorProvider.ts | 35 ++++---- .../notebookStorage/nativeEditorStorage.ts | 82 ++++++------------- .../notebookStorageProvider.ts | 34 ++------ src/client/datascience/types.ts | 22 ++--- .../datascience/export/exportUtil.test.ts | 2 +- .../nativeEditorStorage.unit.test.ts | 20 ++--- .../datascience/mockCustomEditorService.ts | 2 +- .../notebook/contentProvider.ds.test.ts | 2 +- .../notebook/contentProvider.unit.test.ts | 8 +- 16 files changed, 93 insertions(+), 144 deletions(-) create mode 100644 news/2 Fixes/13981.md diff --git a/news/2 Fixes/13981.md b/news/2 Fixes/13981.md new file mode 100644 index 000000000000..265cd21bdb40 --- /dev/null +++ b/news/2 Fixes/13981.md @@ -0,0 +1 @@ +Backup on custom editors is being ignored. \ No newline at end of file diff --git a/src/client/datascience/export/exportUtil.ts b/src/client/datascience/export/exportUtil.ts index 237b93ff4ca1..9e9f2c9c97c8 100644 --- a/src/client/datascience/export/exportUtil.ts +++ b/src/client/datascience/export/exportUtil.ts @@ -57,7 +57,7 @@ export class ExportUtil { await this.jupyterExporter.exportToFile(cells, tempFile.filePath, false); const newPath = path.join(tempDir.path, '.ipynb'); await this.fs.copyLocal(tempFile.filePath, newPath); - model = await this.notebookStorage.getOrCreateModel(Uri.file(newPath)); + model = await this.notebookStorage.getOrCreateModel({ file: Uri.file(newPath) }); } finally { tempFile.dispose(); tempDir.dispose(); @@ -67,7 +67,7 @@ export class ExportUtil { } public async removeSvgs(source: Uri) { - const model = await this.notebookStorage.getOrCreateModel(source); + const model = await this.notebookStorage.getOrCreateModel({ file: source }); const newCells: ICell[] = []; for (const cell of model.cells) { diff --git a/src/client/datascience/interactive-ipynb/nativeEditorProviderOld.ts b/src/client/datascience/interactive-ipynb/nativeEditorProviderOld.ts index 9d7b37ffcd3d..99a857a61834 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditorProviderOld.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditorProviderOld.ts @@ -299,7 +299,7 @@ export class NativeEditorProviderOld extends NativeEditorProvider { this.activeEditors.set(e.file.fsPath, e); // Remove backup storage - this.loadModel(Uri.file(oldPath)) + this.loadModel({ file: Uri.file(oldPath) }) .then((m) => this.storage.deleteBackup(m)) .ignoreErrors(); } @@ -347,7 +347,7 @@ export class NativeEditorProviderOld extends NativeEditorProvider { * I.e. document is already opened in a VSC Notebook. */ private async isDocumentOpenedInVSCodeNotebook(document: TextDocument): Promise { - const model = await this.loadModel(document.uri); + const model = await this.loadModel({ file: document.uri }); // This is temporary code. return model instanceof VSCodeNotebookModel; } diff --git a/src/client/datascience/interactive-ipynb/trustCommandHandler.ts b/src/client/datascience/interactive-ipynb/trustCommandHandler.ts index 3718379cc804..b325032c8fc4 100644 --- a/src/client/datascience/interactive-ipynb/trustCommandHandler.ts +++ b/src/client/datascience/interactive-ipynb/trustCommandHandler.ts @@ -42,7 +42,7 @@ export class TrustCommandHandler implements IExtensionSingleActivationService { return; } - const model = await this.storageProvider.getOrCreateModel(uri); + const model = await this.storageProvider.getOrCreateModel({ file: uri }); if (model.isTrusted) { return; } diff --git a/src/client/datascience/interactive-window/interactiveWindow.ts b/src/client/datascience/interactive-window/interactiveWindow.ts index fd1b2388a161..3dd40ec874cd 100644 --- a/src/client/datascience/interactive-window/interactiveWindow.ts +++ b/src/client/datascience/interactive-window/interactiveWindow.ts @@ -256,7 +256,7 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi break; case InteractiveWindowMessages.ExportNotebookAs: - this.handleMessage(message, payload, this.exportAs); + this.handleMessage(message, payload, this.exportAs.bind); break; case InteractiveWindowMessages.HasCellResponse: diff --git a/src/client/datascience/notebook/contentProvider.ts b/src/client/datascience/notebook/contentProvider.ts index 12e3e0e9d41f..a94bc091a61c 100644 --- a/src/client/datascience/notebook/contentProvider.ts +++ b/src/client/datascience/notebook/contentProvider.ts @@ -70,9 +70,12 @@ export class NotebookContentProvider implements INotebookContentProvider { }; } // If there's no backup id, then skip loading dirty contents. - const model = await (openContext.backupId - ? this.notebookStorage.getOrCreateModel(uri, undefined, openContext.backupId, true) - : this.notebookStorage.getOrCreateModel(uri, undefined, true, true)); + const model = await this.notebookStorage.getOrCreateModel({ + file: uri, + backupId: openContext.backupId, + isNative: true, + skipLoadingDirtyContents: openContext.backupId === undefined + }); if (!(model instanceof VSCodeNotebookModel)) { throw new Error('Incorrect NotebookModel, expected VSCodeNotebookModel'); } @@ -82,7 +85,7 @@ export class NotebookContentProvider implements INotebookContentProvider { } @captureTelemetry(Telemetry.Save, undefined, true) public async saveNotebook(document: NotebookDocument, cancellation: CancellationToken) { - const model = await this.notebookStorage.getOrCreateModel(document.uri, undefined, undefined, true); + const model = await this.notebookStorage.getOrCreateModel({ file: document.uri, isNative: true }); if (cancellation.isCancellationRequested) { return; } @@ -94,7 +97,7 @@ export class NotebookContentProvider implements INotebookContentProvider { document: NotebookDocument, cancellation: CancellationToken ): Promise { - const model = await this.notebookStorage.getOrCreateModel(document.uri, undefined, undefined, true); + const model = await this.notebookStorage.getOrCreateModel({ file: document.uri, isNative: true }); if (!cancellation.isCancellationRequested) { await this.notebookStorage.saveAs(model, targetResource); } @@ -104,7 +107,7 @@ export class NotebookContentProvider implements INotebookContentProvider { _context: NotebookDocumentBackupContext, cancellation: CancellationToken ): Promise { - const model = await this.notebookStorage.getOrCreateModel(document.uri, undefined, undefined, true); + const model = await this.notebookStorage.getOrCreateModel({ file: document.uri, isNative: true }); const id = this.notebookStorage.generateBackupId(model); await this.notebookStorage.backup(model, cancellation, id); return { diff --git a/src/client/datascience/notebook/notebookEditorProvider.ts b/src/client/datascience/notebook/notebookEditorProvider.ts index 6ed78c1a62fa..1cc4b962ff96 100644 --- a/src/client/datascience/notebook/notebookEditorProvider.ts +++ b/src/client/datascience/notebook/notebookEditorProvider.ts @@ -145,7 +145,7 @@ export class NotebookEditorProvider implements INotebookEditorProvider { return; } const uri = doc.uri; - const model = await this.storage.getOrCreateModel(uri, undefined, undefined, true); + const model = await this.storage.getOrCreateModel({ file: uri, isNative: true }); if (model instanceof VSCodeNotebookModel) { model.associateNotebookDocument(doc); } diff --git a/src/client/datascience/notebookStorage/nativeEditorProvider.ts b/src/client/datascience/notebookStorage/nativeEditorProvider.ts index ed9967675ab8..23fcaa04da53 100644 --- a/src/client/datascience/notebookStorage/nativeEditorProvider.ts +++ b/src/client/datascience/notebookStorage/nativeEditorProvider.ts @@ -51,6 +51,7 @@ import { IJupyterDebugger, IJupyterVariableDataProviderFactory, IJupyterVariables, + IModelLoadOptions, INotebookEditor, INotebookEditorProvider, INotebookExporter, @@ -128,22 +129,26 @@ export class NativeEditorProvider implements INotebookEditorProvider, CustomEdit context: CustomDocumentOpenContext, // This has info about backups. right now we use our own data. _cancellation: CancellationToken ): Promise { - const model = await this.loadModel(uri, undefined, context.backupId); + const model = await this.loadModel({ + file: uri, + backupId: context.backupId, + skipLoadingDirtyContents: context.backupId === undefined + }); return { uri, dispose: () => model.dispose() }; } public async saveCustomDocument(document: CustomDocument, cancellation: CancellationToken): Promise { - const model = await this.loadModel(document.uri); + const model = await this.loadModel({ file: document.uri }); return this.storage.save(model, cancellation); } public async saveCustomDocumentAs(document: CustomDocument, targetResource: Uri): Promise { - const model = await this.loadModel(document.uri); + const model = await this.loadModel({ file: document.uri }); return this.storage.saveAs(model, targetResource); } public async revertCustomDocument(document: CustomDocument, cancellation: CancellationToken): Promise { - const model = await this.loadModel(document.uri); + const model = await this.loadModel({ file: document.uri }); return this.storage.revert(model, cancellation); } public async backupCustomDocument( @@ -151,7 +156,7 @@ export class NativeEditorProvider implements INotebookEditorProvider, CustomEdit _context: CustomDocumentBackupContext, cancellation: CancellationToken ): Promise { - const model = await this.loadModel(document.uri); + const model = await this.loadModel({ file: document.uri }); const id = this.storage.generateBackupId(model); await this.storage.backup(model, cancellation, id); return { @@ -167,7 +172,7 @@ export class NativeEditorProvider implements INotebookEditorProvider, CustomEdit public async resolveCustomDocument(document: CustomDocument): Promise { this.customDocuments.set(document.uri.fsPath, document); - await this.loadModel(document.uri); + await this.loadModel({ file: document.uri }); } public async open(file: Uri): Promise { @@ -199,30 +204,26 @@ export class NativeEditorProvider implements INotebookEditorProvider, CustomEdit } @captureTelemetry(Telemetry.CreateNewNotebook, undefined, false) - public async createNew(contents?: string, title?: string): Promise { + public async createNew(possibleContents?: string, title?: string): Promise { // Create a new URI for the dummy file using our root workspace path const uri = this.getNextNewNotebookUri(title); // Set these contents into the storage before the file opens. Make sure not // load from the memento storage though as this is an entirely brand new file. - await this.loadModel(uri, contents, true); + await this.loadModel({ file: uri, possibleContents, skipLoadingDirtyContents: true }); return this.open(uri); } - public async loadModel(file: Uri, contents?: string, skipDirtyContents?: boolean): Promise; - // tslint:disable-next-line: unified-signatures - public async loadModel(file: Uri, contents?: string, backupId?: string): Promise; - // tslint:disable-next-line: no-any - public async loadModel(file: Uri, contents?: string, options?: any): Promise { + public async loadModel(options: IModelLoadOptions): Promise { // Get the model that may match this file - let model = [...this.models.values()].find((m) => this.fs.arePathsSame(m.file, file)); + let model = [...this.models.values()].find((m) => this.fs.arePathsSame(m.file, options.file)); if (!model) { // Every time we load a new untitled file, up the counter past the max value for this counter - this.untitledCounter = getNextUntitledCounter(file, this.untitledCounter); + this.untitledCounter = getNextUntitledCounter(options.file, this.untitledCounter); // Load our model from our storage object. - model = await this.storage.getOrCreateModel(file, contents, options); + model = await this.storage.getOrCreateModel(options); // Make sure to listen to events on the model this.trackModel(model); @@ -273,7 +274,7 @@ export class NativeEditorProvider implements INotebookEditorProvider, CustomEdit protected async loadNotebookEditor(resource: Uri, panel?: WebviewPanel) { try { // Get the model - const model = await this.loadModel(resource); + const model = await this.loadModel({ file: resource }); // Load it (should already be visible) return this.createNotebookEditor(model, panel); diff --git a/src/client/datascience/notebookStorage/nativeEditorStorage.ts b/src/client/datascience/notebookStorage/nativeEditorStorage.ts index 0ca96a90bf9e..f3e71c597de0 100644 --- a/src/client/datascience/notebookStorage/nativeEditorStorage.ts +++ b/src/client/datascience/notebookStorage/nativeEditorStorage.ts @@ -17,6 +17,7 @@ import { CellState, IDataScienceFileSystem, IJupyterExecution, + IModelLoadOptions, INotebookModel, INotebookStorage, ITrustService @@ -75,27 +76,8 @@ export class NativeEditorStorage implements INotebookStorage { public get(_file: Uri): INotebookModel | undefined { return undefined; } - public getOrCreateModel( - file: Uri, - possibleContents?: string, - backupId?: string, - forVSCodeNotebook?: boolean - ): Promise; - public getOrCreateModel( - file: Uri, - possibleContents?: string, - // tslint:disable-next-line: unified-signatures - skipDirtyContents?: boolean, - forVSCodeNotebook?: boolean - ): Promise; - public getOrCreateModel( - file: Uri, - possibleContents?: string, - // tslint:disable-next-line: no-any - options?: any, - forVSCodeNotebook?: boolean - ): Promise { - return this.loadFromFile(file, possibleContents, options, forVSCodeNotebook); + public getOrCreateModel(options: IModelLoadOptions): Promise { + return this.loadFromFile(options); } public async save(model: INotebookModel, _cancellation: CancellationToken): Promise { const contents = model.getContent(); @@ -154,8 +136,8 @@ export class NativeEditorStorage implements INotebookStorage { } public async revert(model: INotebookModel, _cancellation: CancellationToken): Promise { - // Revert to what is in the hot exit file - await this.loadFromFile(model.file); + // Revert to what is in the real file. This is only used for the custom editor + await this.loadFromFile({ file: model.file, skipLoadingDirtyContents: true }); } public async deleteBackup(model: INotebookModel, backupId: string): Promise { @@ -253,54 +235,44 @@ export class NativeEditorStorage implements INotebookStorage { noop(); } } - private loadFromFile( - file: Uri, - possibleContents?: string, - backupId?: string, - forVSCodeNotebook?: boolean - ): Promise; - private loadFromFile( - file: Uri, - possibleContents?: string, - // tslint:disable-next-line: unified-signatures - skipDirtyContents?: boolean, - forVSCodeNotebook?: boolean - ): Promise; - private async loadFromFile( - file: Uri, - possibleContents?: string, - options?: boolean | string, - forVSCodeNotebook?: boolean - ): Promise { + private async loadFromFile(options: IModelLoadOptions): Promise { try { // Attempt to read the contents if a viable file - const contents = NativeEditorStorage.isUntitledFile(file) ? possibleContents : await this.fs.readFile(file); + const contents = NativeEditorStorage.isUntitledFile(options.file) + ? options.possibleContents + : await this.fs.readFile(options.file); - const skipDirtyContents = typeof options === 'boolean' ? options : !!options; - // Use backupId provided, else use static storage key. - const backupId = - typeof options === 'string' ? options : skipDirtyContents ? undefined : this.getStaticStorageKey(file); + // Get backup id from the options if available. + const backupId = options.backupId ? options.backupId : this.getStaticStorageKey(options.file); // If skipping dirty contents, delete the dirty hot exit file now - if (skipDirtyContents) { - await this.clearHotExit(file, backupId); + if (options.skipLoadingDirtyContents) { + await this.clearHotExit(options.file, backupId); } // See if this file was stored in storage prior to shutdown - const dirtyContents = skipDirtyContents ? undefined : await this.getStoredContents(file, backupId); + const dirtyContents = options.skipLoadingDirtyContents + ? undefined + : await this.getStoredContents(options.file, backupId); if (dirtyContents) { // This means we're dirty. Indicate dirty and load from this content - return this.loadContents(file, dirtyContents, true, forVSCodeNotebook); + return this.loadContents(options.file, dirtyContents, true, options.isNative); } else { // Load without setting dirty - return this.loadContents(file, contents, undefined, forVSCodeNotebook); + return this.loadContents(options.file, contents, undefined, options.isNative); } } catch (ex) { // May not exist at this time. Should always have a single cell though - traceError(`Failed to load notebook file ${file.toString()}`, ex); + traceError(`Failed to load notebook file ${options.file.toString()}`, ex); return this.factory.createModel( - { trusted: true, file, cells: [], crypto: this.crypto, globalMemento: this.globalStorage }, - forVSCodeNotebook + { + trusted: true, + file: options.file, + cells: [], + crypto: this.crypto, + globalMemento: this.globalStorage + }, + options.isNative ); } } diff --git a/src/client/datascience/notebookStorage/notebookStorageProvider.ts b/src/client/datascience/notebookStorage/notebookStorageProvider.ts index 4067751614cc..48e28f048c89 100644 --- a/src/client/datascience/notebookStorage/notebookStorageProvider.ts +++ b/src/client/datascience/notebookStorage/notebookStorageProvider.ts @@ -9,7 +9,7 @@ import { CancellationToken } from 'vscode-jsonrpc'; import { IWorkspaceService } from '../../common/application/types'; import { IDisposable, IDisposableRegistry } from '../../common/types'; import { generateNewNotebookUri } from '../common'; -import { INotebookModel, INotebookStorage } from '../types'; +import { IModelLoadOptions, INotebookModel, INotebookStorage } from '../types'; import { getNextUntitledCounter } from './nativeEditorStorage'; import { VSCodeNotebookModel } from './vscNotebookModel'; @@ -66,35 +66,15 @@ export class NotebookStorageProvider implements INotebookStorageProvider { return this.resolvedStorageAndModels.get(file.toString()); } - public getOrCreateModel( - file: Uri, - contents?: string, - backupId?: string, - forVSCodeNotebook?: boolean - ): Promise; - public getOrCreateModel( - file: Uri, - contents?: string, - // tslint:disable-next-line: unified-signatures - skipDirtyContents?: boolean, - forVSCodeNotebook?: boolean - ): Promise; - - public getOrCreateModel( - file: Uri, - contents?: string, - // tslint:disable-next-line: no-any - options?: any, - forVSCodeNotebook?: boolean - ): Promise { - const key = file.toString(); + public getOrCreateModel(options: IModelLoadOptions): Promise { + const key = options.file.toString(); if (!this.storageAndModels.has(key)) { // Every time we load a new untitled file, up the counter past the max value for this counter NotebookStorageProvider.untitledCounter = getNextUntitledCounter( - file, + options.file, NotebookStorageProvider.untitledCounter ); - const promise = this.storage.getOrCreateModel(file, contents, options, forVSCodeNotebook); + const promise = this.storage.getOrCreateModel(options); this.storageAndModels.set(key, promise.then(this.trackModel.bind(this))); } return this.storageAndModels.get(key)!; @@ -105,12 +85,12 @@ export class NotebookStorageProvider implements INotebookStorageProvider { } } - public async createNew(contents?: string, forVSCodeNotebooks?: boolean): Promise { + public async createNew(possibleContents?: string, forVSCodeNotebooks?: boolean): Promise { // Create a new URI for the dummy file using our root workspace path const uri = this.getNextNewNotebookUri(forVSCodeNotebooks); // Always skip loading from the hot exit file. When creating a new file we want a new file. - return this.getOrCreateModel(uri, contents, true, forVSCodeNotebooks); + return this.getOrCreateModel({ file: uri, possibleContents, skipLoadingDirtyContents: true }); } private getNextNewNotebookUri(forVSCodeNotebooks?: boolean): Uri { diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index 3a8b406a24ec..b651af2b7750 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -1106,6 +1106,14 @@ export interface INotebookModel { trust(): void; } +export interface IModelLoadOptions { + isNative?: boolean; + file: Uri; + possibleContents?: string; + backupId?: string; + skipLoadingDirtyContents?: boolean; +} + export const INotebookStorage = Symbol('INotebookStorage'); export interface INotebookStorage { @@ -1114,19 +1122,7 @@ export interface INotebookStorage { saveAs(model: INotebookModel, targetResource: Uri): Promise; backup(model: INotebookModel, cancellation: CancellationToken, backupId?: string): Promise; get(file: Uri): INotebookModel | undefined; - getOrCreateModel( - file: Uri, - contents?: string, - backupId?: string, - forVSCodeNotebook?: boolean - ): Promise; - getOrCreateModel( - file: Uri, - contents?: string, - // tslint:disable-next-line: unified-signatures - skipDirtyContents?: boolean, - forVSCodeNotebook?: boolean - ): Promise; + getOrCreateModel(options: IModelLoadOptions): Promise; revert(model: INotebookModel, cancellation: CancellationToken): Promise; deleteBackup(model: INotebookModel, backupId?: string): Promise; } diff --git a/src/test/datascience/export/exportUtil.test.ts b/src/test/datascience/export/exportUtil.test.ts index c59d133f3feb..1e80292e1829 100644 --- a/src/test/datascience/export/exportUtil.test.ts +++ b/src/test/datascience/export/exportUtil.test.ts @@ -37,7 +37,7 @@ suite('DataScience - Export Util', () => { ); await exportUtil.removeSvgs(file); - const model = await notebookStorage.getOrCreateModel(file); + const model = await notebookStorage.getOrCreateModel({ file }); // make sure no svg exists in model const SVG = 'image/svg+xml'; diff --git a/src/test/datascience/interactive-ipynb/nativeEditorStorage.unit.test.ts b/src/test/datascience/interactive-ipynb/nativeEditorStorage.unit.test.ts index ad5093ca7361..103d2d40b8ca 100644 --- a/src/test/datascience/interactive-ipynb/nativeEditorStorage.unit.test.ts +++ b/src/test/datascience/interactive-ipynb/nativeEditorStorage.unit.test.ts @@ -425,7 +425,7 @@ suite('DataScience - Native Editor Storage', () => { } test('Create new editor and add some cells', async () => { - model = await storage.getOrCreateModel(baseUri); + model = await storage.getOrCreateModel({ file: baseUri }); insertCell(0, '1'); const cells = model.cells; expect(cells).to.be.lengthOf(4); @@ -434,7 +434,7 @@ suite('DataScience - Native Editor Storage', () => { }); test('Move cells around', async () => { - model = await storage.getOrCreateModel(baseUri); + model = await storage.getOrCreateModel({ file: baseUri }); swapCells('NotebookImport#0', 'NotebookImport#1'); const cells = model.cells; expect(cells).to.be.lengthOf(3); @@ -443,7 +443,7 @@ suite('DataScience - Native Editor Storage', () => { }); test('Edit/delete cells', async () => { - model = await storage.getOrCreateModel(baseUri); + model = await storage.getOrCreateModel({ file: baseUri }); expect(model.isDirty).to.be.equal(false, 'Editor should not be dirty'); editCell( [ @@ -483,7 +483,7 @@ suite('DataScience - Native Editor Storage', () => { test('Editing a file and closing will keep contents', async () => { await filesConfig?.update('autoSave', 'off'); - model = await storage.getOrCreateModel(baseUri); + model = await storage.getOrCreateModel({ file: baseUri }); expect(model.isDirty).to.be.equal(false, 'Editor should not be dirty'); editCell( [ @@ -512,7 +512,7 @@ suite('DataScience - Native Editor Storage', () => { // Recreate storage = createStorage(); - model = await storage.getOrCreateModel(baseUri); + model = await storage.getOrCreateModel({ file: baseUri }); const cells = model.cells; expect(cells).to.be.lengthOf(3); @@ -522,7 +522,7 @@ suite('DataScience - Native Editor Storage', () => { }); test('Editing a new file and closing will keep contents', async () => { - model = await storage.getOrCreateModel(untiledUri, undefined, true); + model = await storage.getOrCreateModel({ file: untiledUri, skipLoadingDirtyContents: true }); expect(model.isDirty).to.be.equal(false, 'Editor should not be dirty'); insertCell(0, 'a=1'); @@ -531,7 +531,7 @@ suite('DataScience - Native Editor Storage', () => { // Recreate storage = createStorage(); - model = await storage.getOrCreateModel(untiledUri); + model = await storage.getOrCreateModel({ file: untiledUri }); const cells = model.cells; expect(cells).to.be.lengthOf(2); @@ -550,7 +550,7 @@ suite('DataScience - Native Editor Storage', () => { // Put the regular file into the local storage await localMemento.update(`notebook-storage-${file.toString()}`, differentFile); - model = await storage.getOrCreateModel(file); + model = await storage.getOrCreateModel({ file }); // It should load with that value const cells = model.cells; @@ -571,7 +571,7 @@ suite('DataScience - Native Editor Storage', () => { contents: differentFile, lastModifiedTimeMs: Date.now() }); - model = await storage.getOrCreateModel(file); + model = await storage.getOrCreateModel({ file }); // It should load with that value const cells = model.cells; @@ -601,7 +601,7 @@ suite('DataScience - Native Editor Storage', () => { lastModifiedTimeMs: Date.now() }); - model = await storage.getOrCreateModel(file); + model = await storage.getOrCreateModel({ file }); // It should load with that value const cells = model.cells; diff --git a/src/test/datascience/mockCustomEditorService.ts b/src/test/datascience/mockCustomEditorService.ts index 03ceee2b8c25..fa59c744d1b0 100644 --- a/src/test/datascience/mockCustomEditorService.ts +++ b/src/test/datascience/mockCustomEditorService.ts @@ -138,7 +138,7 @@ export class MockCustomEditorService implements ICustomEditorService { private async getModel(file: Uri): Promise { const nativeProvider = this.provider as NativeEditorProvider; if (nativeProvider) { - return (nativeProvider.loadModel(file) as unknown) as Promise; + return (nativeProvider.loadModel({ file }) as unknown) as Promise; } return undefined; } diff --git a/src/test/datascience/notebook/contentProvider.ds.test.ts b/src/test/datascience/notebook/contentProvider.ds.test.ts index d2a64d11ef61..fa283388337f 100644 --- a/src/test/datascience/notebook/contentProvider.ds.test.ts +++ b/src/test/datascience/notebook/contentProvider.ds.test.ts @@ -60,7 +60,7 @@ suite('DataScience - VSCode Notebook - (Open)', function () { 'notebook', 'testJsonContents.ipynb' ); - const model = await storageProvider.getOrCreateModel(Uri.file(file)); + const model = await storageProvider.getOrCreateModel({ file: Uri.file(file) }); disposables.push(model); model.trust(); const jsonStr = fs.readFileSync(file, { encoding: 'utf8' }); diff --git a/src/test/datascience/notebook/contentProvider.unit.test.ts b/src/test/datascience/notebook/contentProvider.unit.test.ts index 39fd8e5b3464..5ccae9026d79 100644 --- a/src/test/datascience/notebook/contentProvider.unit.test.ts +++ b/src/test/datascience/notebook/contentProvider.unit.test.ts @@ -55,9 +55,7 @@ suite('DataScience - NativeNotebook ContentProvider', () => { ] } ); - when(storageProvider.getOrCreateModel(anything(), anything(), anything(), anything())).thenResolve( - model - ); + when(storageProvider.getOrCreateModel(anything())).thenResolve(model); const notebook = await contentProvider.openNotebook(fileUri, {}); @@ -135,9 +133,7 @@ suite('DataScience - NativeNotebook ContentProvider', () => { ] } ); - when(storageProvider.getOrCreateModel(anything(), anything(), anything(), anything())).thenResolve( - model - ); + when(storageProvider.getOrCreateModel(anything())).thenResolve(model); const notebook = await contentProvider.openNotebook(fileUri, {}); From 5ffcb4b39ce078009a2f22421b94c5ef45b2049e Mon Sep 17 00:00:00 2001 From: rchiodo Date: Fri, 18 Sep 2020 10:12:46 -0700 Subject: [PATCH 3/3] Fixup changelog --- CHANGELOG.md | 2 ++ news/2 Fixes/13981.md | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) delete mode 100644 news/2 Fixes/13981.md diff --git a/CHANGELOG.md b/CHANGELOG.md index cae1f6150600..38b5f71f1330 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,8 @@ ([#13706](https://github.com/Microsoft/vscode-python/issues/13706)) 1. Correctly install ipykernel when launching from an interpreter. ([#13956](https://github.com/Microsoft/vscode-python/issues/13956)) +1. Backup on custom editors is being ignored. + ([#13981](https://github.com/Microsoft/vscode-python/issues/13981)) ### Code Health diff --git a/news/2 Fixes/13981.md b/news/2 Fixes/13981.md deleted file mode 100644 index 265cd21bdb40..000000000000 --- a/news/2 Fixes/13981.md +++ /dev/null @@ -1 +0,0 @@ -Backup on custom editors is being ignored. \ No newline at end of file