Fix storage not being used

news/2 Fixes/11557.md

@@ -0,0 +1 @@
+When VS quits, make sure to save contents of notebook for next reopen.

src/client/datascience/interactive-ipynb/nativeEditorStorage.ts

@@ -19,6 +19,7 @@ import { CellState, ICell, IJupyterExecution, IJupyterKernelSpec, INotebookModel
 
 // tslint:disable-next-line:no-require-imports no-var-requires
 import detectIndent = require('detect-indent');
+import { IWorkspaceService } from '../../common/application/types';
 import { sendTelemetryEvent } from '../../telemetry';
 import { pruneCell } from '../common';
 // tslint:disable-next-line:no-require-imports no-var-requires
@@ -74,7 +75,8 @@ export class NativeEditorStorage implements INotebookModel, INotebookStorage {
         @inject(ICryptoUtils) private crypto: ICryptoUtils,
         @inject(IExtensionContext) private context: IExtensionContext,
         @inject(IMemento) @named(GLOBAL_MEMENTO) private globalStorage: Memento,
-        @inject(IMemento) @named(WORKSPACE_MEMENTO) private localStorage: Memento
+        @inject(IMemento) @named(WORKSPACE_MEMENTO) private localStorage: Memento,
+        @inject(IWorkspaceService) private workspaceService: IWorkspaceService
     ) {}
 
     public async load(file: Uri, possibleContents?: string): Promise<INotebookModel> {
@@ -141,6 +143,17 @@ export class NativeEditorStorage implements INotebookModel, INotebookStorage {
         // Write but debounced (wait at least 250 ms)
         return this.debouncedWriteToStorage(filePath, specialContents, cancelToken);
     }
+
+    private async saveToStorage(): Promise<void> {
+        // Skip doing this if auto save is enabled or is untitled
+        const filesConfig = this.workspaceService.getConfiguration('files', this.file);
+        const autoSave = filesConfig.get('autoSave', 'off');
+        if (autoSave === 'off' && !this.isUntitled) {
+            // Write but debounced (wait at least 250 ms)
+            return this.storeContentsInHotExitFile(undefined);
+        }
+    }
+
     private async writeToStorage(filePath: string, contents?: string, cancelToken?: CancellationToken): Promise<void> {
         try {
             if (!cancelToken?.isCancellationRequested) {
@@ -198,6 +211,10 @@ export class NativeEditorStorage implements INotebookModel, INotebookStorage {
         // Forward onto our listeners if necessary
         if (changed || this.isDirty !== oldDirty) {
             this._changedEmitter.fire({ ...change, newDirty: this.isDirty, oldDirty });
+            if (this.isDirty) {
+                // Save to temp storage so we don't lose the file if the user exits VS code
+                this.saveToStorage().ignoreErrors();
+            }
         }
         // Slightly different for the event we send to VS code. Skip version and file changes. Only send user events.
         if (
@@ -451,6 +468,7 @@ export class NativeEditorStorage implements INotebookModel, INotebookStorage {
             const dirtyContents = await this.getStoredContents();
             if (dirtyContents) {
                 // This means we're dirty. Indicate dirty and load from this content
+                this._state.saveChangeCount = -1; // Indicates dirty
                 this.loadContents(dirtyContents);
             } else {
                 // Load without setting dirty
@@ -621,8 +639,8 @@ export class NativeEditorStorage implements INotebookModel, INotebookStorage {
             if (data && !this.isUntitled && data.contents) {
                 return data.contents;
             }
-        } catch {
-            noop();
+        } catch (exc) {
+            traceError(`Exception reading from temporary storage for ${key}`, exc);
         }
     }
 

src/test/datascience/interactive-ipynb/nativeEditorStorage.unit.test.ts

@@ -11,6 +11,7 @@ import {
     ConfigurationChangeEvent,
     ConfigurationTarget,
     EventEmitter,
+    FileType,
     TextEditor,
     Uri,
     WorkspaceConfiguration
@@ -31,6 +32,7 @@ import { ConfigurationService } from '../../../client/common/configuration/servi
 import { CryptoUtils } from '../../../client/common/crypto';
 import { IFileSystem } from '../../../client/common/platform/types';
 import { IConfigurationService, ICryptoUtils, IDisposable, IExtensionContext } from '../../../client/common/types';
+import { sleep } from '../../../client/common/utils/async';
 import { EXTENSION_ROOT_DIR } from '../../../client/constants';
 import {
     IEditorContentChange,
@@ -321,6 +323,7 @@ suite('Data Science - Native Editor Storage', () => {
 
         let listener: IWebPanelMessageListener;
         const webPanel = mock(WebPanel);
+        const startTime = Date.now();
         class WebPanelCreateMatcher extends Matcher {
             public match(value: any) {
                 listener = value.listener;
@@ -351,16 +354,25 @@ suite('Data Science - Native Editor Storage', () => {
             .returns((_a1) => {
                 return Promise.resolve(lastWriteFileValue);
             });
+        fileSystem
+            .setup((f) => f.stat(typemoq.It.isAny()))
+            .returns((_a1) => {
+                return Promise.resolve({ mtime: startTime, type: FileType.File, ctime: startTime, size: 100 });
+            });
+        storage = createStorage();
+    });
 
-        storage = new NativeEditorStorage(
+    function createStorage() {
+        return new NativeEditorStorage(
             instance(executionProvider),
             fileSystem.object, // Use typemoq so can save values in returns
             instance(crypto),
             context.object,
             globalMemento,
-            localMemento
+            localMemento,
+            instance(workspace)
         );
-    });
+    }
 
     teardown(() => {
         globalMemento.clear();
@@ -480,6 +492,47 @@ suite('Data Science - Native Editor Storage', () => {
         expect(cells).to.be.lengthOf(1);
     });
 
+    test('Editing a file and closing will keep contents', async () => {
+        await filesConfig?.update('autoSave', 'off');
+
+        await storage.load(baseUri);
+        expect(storage.isDirty).to.be.equal(false, 'Editor should not be dirty');
+        editCell(
+            [
+                {
+                    range: {
+                        startLineNumber: 2,
+                        startColumn: 1,
+                        endLineNumber: 2,
+                        endColumn: 1
+                    },
+                    rangeOffset: 4,
+                    rangeLength: 0,
+                    text: 'a',
+                    position: {
+                        lineNumber: 1,
+                        column: 1
+                    }
+                }
+            ],
+            storage.cells[1],
+            'a'
+        );
+
+        // Wait for a second so it has time to update
+        await sleep(1000);
+
+        // Recreate
+        storage = createStorage();
+        await storage.load(baseUri);
+
+        const cells = storage.cells;
+        expect(cells).to.be.lengthOf(3);
+        expect(cells[1].id).to.be.match(/NotebookImport#1/);
+        expect(concatMultilineStringInput(cells[1].data.source)).to.be.equals('b=2\nab');
+        expect(storage.isDirty).to.be.equal(true, 'Editor should be dirty');
+    });
+
     test('Opening file with local storage but no global will still open with old contents', async () => {
         await filesConfig?.update('autoSave', 'off');
         // This test is really for making sure when a user upgrades to a new extension, we still have their old storage