Repository: stateful/vscode-runme
Pull request #642 (2023-06-26 13:21:57)

Presave check is no longer required
It dates back to when the serializer part of the parser was "experimental".

Also, tests coupled with WASM Serializer are obsolete. Serialization testing is now covered in https://github.com/stateful/runme.

PS: Relevant code makes saving fail when the markdown diff editor is active.


Patch:
diff --git a/src/extension/serializer.ts b/src/extension/serializer.ts
index 3479ffee..06f40398 100644
--- a/src/extension/serializer.ts
+++ b/src/extension/serializer.ts
@@ -1,6 +1,4 @@
 import {
-  window,
-  commands,
   NotebookSerializer,
   ExtensionContext,
   Uri,
@@ -26,7 +24,7 @@ import { DeserializeRequest, SerializeRequest, Notebook } from './grpc/serialize
 import { initParserClient, ParserServiceClient } from './grpc/client'
 import Languages from './languages'
 import { PLATFORM_OS } from './constants'
-import { canEditFile, initWasm } from './utils'
+import { initWasm } from './utils'
 import RunmeServer from './server/runmeServer'
 import { Kernel } from './kernel'
 
@@ -128,8 +126,6 @@ export abstract class SerializerBase implements NotebookSerializer, Disposable {
     // eslint-disable-next-line @typescript-eslint/no-unused-vars
     token: CancellationToken
   ): Promise<Uint8Array> {
-    await this.preSaveCheck()
-
     const transformedCells = data.cells.map((cell) => {
       return {
         ...cell,
@@ -213,34 +209,6 @@ export abstract class SerializerBase implements NotebookSerializer, Disposable {
     return notebookData
   }
 
-  protected async preSaveCheck() {
-    if (!window.activeNotebookEditor) {
-      throw new Error("Could't save notebook as it is not active!")
-    }
-
-    if (!(await canEditFile(window.activeNotebookEditor.notebook))) {
-      const errorMessage =
-        'You are writing to a file that is not version controlled! ' +
-        "Runme's authoring features are in early stages and require hardening. " +
-        "We wouldn't want you to loose important data. Please version track your file first " +
-        'or disable this restriction in the VS Code settings.'
-      window.showErrorMessage(errorMessage, 'Open Runme Settings').then((openSettings) => {
-        if (openSettings) {
-          return commands.executeCommand(
-            'workbench.action.openSettings',
-            'runme.flags.disableSaveRestriction'
-          )
-        }
-      })
-      throw new Error('saving non version controlled notebooks is disabled by default.')
-    }
-
-    const err = await this.ready
-    if (err) {
-      throw err
-    }
-  }
-
   protected static revive(notebook: Serializer.Notebook) {
     return notebook.cells.reduce((accu, elem) => {
       let cell: NotebookCellData
diff --git a/src/extension/utils.ts b/src/extension/utils.ts
index fc0695e2..45b2cbc4 100644
--- a/src/extension/utils.ts
+++ b/src/extension/utils.ts
@@ -8,7 +8,6 @@ import vscode, {
   Uri,
   workspace,
   env,
-  NotebookDocument,
   NotebookCell,
   NotebookCellExecution,
   NotebookCellOutput,
@@ -292,40 +291,6 @@ export async function verifyCheckedInFile(filePath: string) {
   return isCheckedIn
 }
 
-export async function canEditFile(
-  notebook: NotebookDocument,
-  // for testing purposes only
-  verifyCheckedInFileFn = verifyCheckedInFile
-): Promise<boolean> {
-  const config = vscode.workspace.getConfiguration('runme.flags')
-  const disableSaveRestriction = config.get<boolean>('disableSaveRestriction')
-  const currentDocumentPath = notebook.uri.fsPath
-  const isNewFile = notebook.isUntitled && notebook.notebookType === Kernel.type
-
-  /**
-   * allow serializing files if:
-   */
-  if (
-    /**
-     * the user has disabled this restriction
-     */
-    disableSaveRestriction ||
-    /**
-     * the user just created a new file
-     */
-    isNewFile ||
-    /**
-     * the user works on a checked in file
-     */
-    !currentDocumentPath ||
-    (await verifyCheckedInFileFn(currentDocumentPath))
-  ) {
-    return true
-  }
-
-  return false
-}
-
 export async function initWasm(wasmUri: Uri) {
   const go = new globalThis.Go()
   const wasmFile = await workspace.fs.readFile(wasmUri)


Test patch:
diff --git a/tests/extension/serializer.test.ts b/tests/extension/serializer.test.ts
index 1e99114d..e7ce4227 100644
--- a/tests/extension/serializer.test.ts
+++ b/tests/extension/serializer.test.ts
@@ -2,7 +2,6 @@ import { NotebookData, NotebookDocument, NotebookEdit, window, workspace } from
 import { expect, vi, it, describe, beforeEach } from 'vitest'
 
 import { SerializerBase, WasmSerializer } from '../../src/extension/serializer'
-import { canEditFile } from '../../src/extension/utils'
 import type { Kernel } from '../../src/extension/kernel'
 import { EventEmitter, Uri } from '../../__mocks__/vscode'
 import { Serializer } from '../../src/types'
@@ -44,7 +43,6 @@ vi.mock('../../src/extension/languages', () => ({
 }))
 
 vi.mock('../../src/extension/utils', () => ({
-    canEditFile: vi.fn().mockResolvedValue(false),
     initWasm: vi.fn()
 }))
 
@@ -163,34 +161,9 @@ describe('WasmSerializer', () => {
     }
 
     describe('serializeNotebook', () => {
-        it('fails when notebook is not active', async () => {
-            const s = new WasmSerializer(context, newKernel())
-            await expect(() => s.serializeNotebook({} as any, {} as any))
-                .rejects.toThrow(/not active/)
-        })
-
-        it('prevents saving if canEditFile returns false', async () => {
-            // @ts-ignore readonly
-            window.activeNotebookEditor = {} as any
-            const s = new WasmSerializer(context, newKernel())
-            await expect(() => s.serializeNotebook({} as any, {} as any))
-                .rejects.toThrow(/saving non version controlled notebooks is disabled/)
-        })
-
-        it('throws if wasm fails to laod', async () => {
-            // @ts-ignore readonly
-            window.activeNotebookEditor = {} as any
-            vi.mocked(canEditFile).mockResolvedValue(true)
-            const s = new WasmSerializer(context, newKernel())
-            // @ts-ignore readonly
-            s['ready'] = Promise.reject('ups')
-            await expect(() => s.serializeNotebook({} as any, {} as any)).rejects.toThrow(/ups/)
-        })
-
         it('uses Runme wasm to save the file', async () => {
             // @ts-ignore readonly
             window.activeNotebookEditor = {} as any
-            vi.mocked(canEditFile).mockResolvedValue(true)
             const s = new WasmSerializer(context, newKernel())
             // @ts-ignore readonly
             s['ready'] = Promise.resolve()
diff --git a/tests/extension/utils.test.ts b/tests/extension/utils.test.ts
index c606913f..f7114de5 100644
--- a/tests/extension/utils.test.ts
+++ b/tests/extension/utils.test.ts
@@ -8,7 +8,6 @@ import {
   getKey,
   getCmdShellSeq,
   normalizeLanguage,
-  canEditFile,
   getAnnotations,
   mapGitIgnoreToGlobFolders,
   hashDocumentUri,
@@ -212,45 +211,6 @@ suite('normalizeLanguage', () => {
   })
 })
 
-suite('canEditFile', () => {
-  const verifyCheckedInFile = vi.fn().mockResolvedValue(false)
-  const notebook: any = {
-    isUntitled: false,
-    notebookType: 'runme',
-    uri: { fsPath: '/foo/bar' }
-  }
-
-  beforeEach(() => {
-    vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
-      get: vi.fn().mockReturnValue(false)
-    } as any)
-  })
-
-  test('can not edit by default', async () => {
-    expect(await canEditFile(notebook, verifyCheckedInFile)).toBe(false)
-  })
-
-  test('can edit if ignore flag is enabled', async () => {
-    const notebookMock: any = JSON.parse(JSON.stringify(notebook))
-    vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
-      get: vi.fn().mockReturnValue(true)
-    } as any)
-    expect(await canEditFile(notebookMock, verifyCheckedInFile)).toBe(true)
-  })
-
-  test('can edit file if new', async () => {
-    const notebookMock: any = JSON.parse(JSON.stringify(notebook))
-    notebookMock.isUntitled = true
-    expect(await canEditFile(notebookMock, verifyCheckedInFile)).toBe(true)
-  })
-
-  test('can edit file if checked in', async () => {
-    const notebookMock: any = JSON.parse(JSON.stringify(notebook))
-    verifyCheckedInFile.mockResolvedValue(true)
-    expect(await canEditFile(notebookMock, verifyCheckedInFile)).toBe(true)
-  })
-})
-
 suite('mapGitIgnoreToGlobFolders', () => {
   test('map properly to glob patterns folders', () => {
     const gitIgnoreContents = `