Repository: stateful/vscode-runme
Pull request #593 (2023-06-07 22:09:53)

Run CLI with index rather than cell name
Requires stateful/runme#293

CLI button now uses cell index rather than cell name. Note that this feature will break for users who don't upgrade to the latest runme.

This unblocks us on [this issue](https://github.com/stateful/vscode-runme/issues/566#issuecomment-1574128419), though it doesn't solve it.

I also introduced `runme.cli.useIntegratedRunme`, which makes it easier to test the new version of runme (uses integrated path).


Patch:
diff --git a/package-lock.json b/package-lock.json
index 395d4363..373e1485 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,12 +1,12 @@
 {
   "name": "runme",
-  "version": "1.5.1",
+  "version": "1.5.2-edge.1",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {
     "": {
       "name": "runme",
-      "version": "1.5.1",
+      "version": "1.5.2-edge.1",
       "hasInstallScript": true,
       "license": "Apache-2.0",
       "dependencies": {
diff --git a/package.json b/package.json
index b56c3ead..1bd147ff 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "runme",
-  "version": "1.5.1",
+  "version": "1.5.2-edge.1",
   "publisher": "stateful",
   "displayName": "Runme",
   "description": "Execute commands inside your runbooks, docs, and READMEs.",
@@ -425,6 +425,11 @@
             "type": "boolean",
             "default": true,
             "markdownDescription": "Load environment files from workspace into all scripts"
+          },
+          "runme.cli.useIntegratedRunme": {
+            "type": "boolean",
+            "default": false,
+            "markdownDescription": "Whether to use full path to integrated runme executable when running CLI commands. This is mostly useful for development purposes."
           }
         }
       }
diff --git a/src/extension/commands/index.ts b/src/extension/commands/index.ts
index c2469222..d35af58f 100644
--- a/src/extension/commands/index.ts
+++ b/src/extension/commands/index.ts
@@ -18,12 +18,21 @@ import {
 } from 'vscode'
 import { v4 as uuidv4 } from 'uuid'
 
-import { getTLSDir, getTLSEnabled, isNotebookTerminalEnabledForCell } from '../../utils/configuration'
+import {
+  getBinaryPath,
+  getCLIUseIntegratedRunme,
+  getTLSDir,
+  getTLSEnabled,
+  isNotebookTerminalEnabledForCell
+} from '../../utils/configuration'
 import { Kernel } from '../kernel'
 import { getAnnotations, getNotebookCategories, getTerminalByCell, openFileAsRunmeNotebook } from '../utils'
 import RunmeServer from '../server/runmeServer'
 import { GrpcRunnerEnvironment } from '../runner'
 import { NotebookToolbarCommand } from '../../types'
+import getLogger from '../logger'
+
+const log = getLogger('Commands')
 
 function showWarningMessage() {
   return window.showWarningMessage('Couldn\'t find terminal! Was it already closed?')
@@ -116,12 +125,38 @@ export function runCLICommand(
   server: RunmeServer,
   kernel: Kernel
 ) {
-  return async function (cell: { metadata?: any, document: TextDocument }) {
+  return async function (cell: NotebookCell) {
+    if (cell.notebook.isDirty) {
+      const option = await window.showInformationMessage(
+        'You have unsaved changes. Save and run in CLI?',
+        'Save',
+        'Cancel'
+      )
+
+      if (option === 'Cancel' || !option) {
+        return
+      }
+
+      await cell.notebook.save()
+    }
+
     const cwd = path.dirname(cell.document.uri.fsPath)
 
+    const index = cell.notebook.getCells()
+      .filter(x => x.kind === NotebookCellKind.Code)
+      .indexOf(cell)
+
+    if (index < 0) {
+      window.showErrorMessage('Internal error identifying cell index')
+      log.error(`Failed getting code cell index for cell at index ${cell.index}`)
+
+      return
+    }
+
     const args = [
       `--chdir="${cwd}"`,
-      `--filename="${path.basename(cell.document.uri.fsPath)}"`
+      `--filename="${path.basename(cell.document.uri.fsPath)}"`,
+      `--index=${index}`
     ]
 
     const envs: Record<string, string> = {}
@@ -148,8 +183,10 @@ export function runCLICommand(
       env: envs,
     })
 
+    const runmeExec = getCLIUseIntegratedRunme() ? getBinaryPath(extensionBaseUri, process.platform).fsPath : 'runme'
+
     term.show(false)
-    term.sendText(`runme run ${annotations.name} ${args.join(' ')}`)
+    term.sendText(`${runmeExec} run ${args.join(' ')}`)
   }
 }
 
diff --git a/src/extension/serializer.ts b/src/extension/serializer.ts
index b4ecbcd2..8ae72841 100644
--- a/src/extension/serializer.ts
+++ b/src/extension/serializer.ts
@@ -48,7 +48,6 @@ export abstract class SerializerBase implements NotebookSerializer, Disposable {
       workspace.onDidChangeNotebookDocument(
         this.handleNotebookChanged.bind(this)
       ),
-      // TODO(mxs): https://github.com/stateful/vscode-runme/issues/566#issuecomment-1574128419
       // workspace.onDidSaveNotebookDocument(
       //   this.handleNotebookSaved.bind(this)
       // )
diff --git a/src/utils/configuration.ts b/src/utils/configuration.ts
index c8a16efa..2e0e994b 100644
--- a/src/utils/configuration.ts
+++ b/src/utils/configuration.ts
@@ -12,6 +12,7 @@ const SERVER_SECTION_NAME = 'runme.server'
 const TERMINAL_SECTION_NAME = 'runme.terminal'
 const CODELENS_SECTION_NAME = 'runme.codelens'
 const ENV_SECTION_NAME = 'runme.env'
+const CLI_SECTION_NAME = 'runme.cli'
 
 export const DEFAULT_TLS_DIR = path.join(os.tmpdir(), 'runme', uuidv4(), 'tls')
 const DEFAULT_WORKSPACE_FILE_ORDER = ['.env.local', '.env']
@@ -52,6 +53,9 @@ const configurationSchema = {
     env: {
       workspaceFileOrder: z.array(z.string()).default(DEFAULT_WORKSPACE_FILE_ORDER),
       loadWorkspaceFiles: z.boolean().default(true),
+    },
+    cli: {
+      useIntegratedRunme: z.boolean().default(false),
     }
 }
 
@@ -95,6 +99,16 @@ const getEnvConfigurationValue = <T>(configName: keyof typeof configurationSchem
   return defaultValue
 }
 
+const getCLIConfigurationValue = <T>(configName: keyof typeof configurationSchema.cli, defaultValue: T) => {
+  const configurationSection = workspace.getConfiguration(CLI_SECTION_NAME)
+  const configurationValue = configurationSection.get<T>(configName)!
+  const parseResult = configurationSchema.cli[configName].safeParse(configurationValue)
+  if (parseResult.success) {
+      return parseResult.data as T
+  }
+  return defaultValue
+}
+
 const getPortNumber = (): number => {
   return SERVER_PORT
 }
@@ -185,6 +199,10 @@ const getEnvLoadWorkspaceFiles = (): boolean => {
   return getEnvConfigurationValue('loadWorkspaceFiles', true)
 }
 
+const getCLIUseIntegratedRunme = (): boolean => {
+  return getCLIConfigurationValue('useIntegratedRunme', false)
+}
+
 export {
     getPortNumber,
     getBinaryPath,
@@ -202,4 +220,5 @@ export {
     getCustomServerAddress,
     getEnvWorkspaceFileOrder,
     getEnvLoadWorkspaceFiles,
+    getCLIUseIntegratedRunme,
 }


Test patch:
diff --git a/tests/extension/commands/index.test.ts b/tests/extension/commands/index.test.ts
index bcacc6d0..32f66380 100644
--- a/tests/extension/commands/index.test.ts
+++ b/tests/extension/commands/index.test.ts
@@ -1,6 +1,6 @@
 import path from 'node:path'
 
-import { beforeEach, expect, test, vi } from 'vitest'
+import { beforeEach, expect, test, vi, suite } from 'vitest'
 import {
   window,
   env,
@@ -28,7 +28,11 @@ import {
   openFileInRunme
 } from '../../../src/extension/commands'
 import { getTerminalByCell, getAnnotations, openFileAsRunmeNotebook } from '../../../src/extension/utils'
-import { getBinaryPath, isNotebookTerminalEnabledForCell } from '../../../src/utils/configuration'
+import {
+  getBinaryPath,
+  getCLIUseIntegratedRunme,
+  isNotebookTerminalEnabledForCell
+} from '../../../src/utils/configuration'
 
 vi.mock('vscode', () => import(path.join(process.cwd(), '__mocks__', 'vscode')))
 vi.mock('vscode-telemetry')
@@ -41,6 +45,7 @@ vi.mock('../../../src/extension/utils', () => ({
 vi.mock('../../../src/utils/configuration', () => ({
   getBinaryPath: vi.fn(),
   isNotebookTerminalEnabledForCell: vi.fn(),
+  getCLIUseIntegratedRunme: vi.fn().mockReturnValue(false),
 }))
 vi.mock('../../../src/extension/provider/cli', () => ({
   CliProvider: {
@@ -106,21 +111,96 @@ test('copyCellToClipboard', () => {
   expect(window.showInformationMessage).toBeCalledTimes(1)
 })
 
-test('runCLICommand', async () => {
-  const cell: any = {
-    metadata: { name: 'foobar' },
-    document: { uri: { fsPath: '/foo/bar/README.md' }}
-  }
-  vi.mocked(getAnnotations).mockReturnValueOnce({
-    name: 'foo-bar',
-  } as any)
-  vi.mocked(getBinaryPath).mockReturnValueOnce(Uri.file('/bin/runme'))
-  vi.mocked(window.showInformationMessage).mockResolvedValueOnce(false as any)
-
-  await runCLICommand({} as any, false, {} as any, {} as any)(cell)
-  expect(vi.mocked((terminal as any).sendText)).toHaveBeenCalledWith(
-    'runme run foo-bar --chdir="/foo/bar" --filename="README.md"'
-  )
+suite('runCliCommand', () => {
+  beforeEach(() => {
+    vi.mocked(getBinaryPath).mockClear()
+    vi.mocked(window.showInformationMessage).mockClear()
+    vi.mocked(getAnnotations).mockClear()
+    vi.mocked(terminal.sendText).mockClear()
+    vi.mocked(getCLIUseIntegratedRunme).mockClear()
+  })
+
+  test('runs command when not dirty, using index', async () => {
+    const cell: any = {
+      metadata: { name: 'foobar' },
+      document: { uri: { fsPath: '/foo/bar/README.md' }},
+      kind: 1
+    }
+
+    cell.notebook = {
+      getCells: () => [cell],
+      isDirty: false
+    }
+
+    // vi.mocked(getAnnotations).mockReturnValueOnce({
+    //   name: 'foo-bar',
+    // } as any)
+
+    // vi.mocked(getBinaryPath).mockReturnValueOnce(Uri.file('/bin/runme'))
+    // vi.mocked(window.showInformationMessage).mockResolvedValueOnce(false as any)
+
+    await runCLICommand({} as any, false, {} as any, {} as any)(cell)
+    expect(vi.mocked((terminal as any).sendText)).toHaveBeenCalledWith(
+      'runme run --chdir="/foo/bar" --filename="README.md" --index=0'
+    )
+  })
+
+  test('uses integrated runme if config set', async () => {
+    const cell: any = {
+      metadata: { name: 'foobar' },
+      document: { uri: { fsPath: '/foo/bar/README.md' }},
+      kind: 1
+    }
+
+    cell.notebook = {
+      getCells: () => [cell],
+      isDirty: false
+    }
+
+    vi.mocked(getBinaryPath).mockReturnValueOnce(Uri.file('/bin/runme'))
+    vi.mocked(getCLIUseIntegratedRunme).mockReturnValueOnce(true)
+
+    await runCLICommand({} as any, false, {} as any, {} as any)(cell)
+    expect(vi.mocked((terminal as any).sendText)).toHaveBeenCalledWith(
+      '/bin/runme run --chdir="/foo/bar" --filename="README.md" --index=0'
+    )
+  })
+
+  test('prompts user when dirty and fails if cancelled or closed', async () => {
+    const cell: any = {
+      metadata: { name: 'foobar' },
+      document: { uri: { fsPath: '/foo/bar/README.md' }},
+      kind: 1
+    }
+
+    cell.notebook = {
+      getCells: () => [cell],
+      isDirty: true,
+      save: vi.fn()
+    }
+
+    // Cancelled
+    vi.mocked(window.showInformationMessage).mockResolvedValueOnce('Cancel' as any)
+    await runCLICommand({} as any, false, {} as any, {} as any)(cell)
+    expect(vi.mocked(terminal.sendText)).not.toHaveBeenCalled()
+
+    vi.mocked(terminal.sendText).mockClear()
+
+    // Closed
+    vi.mocked(window.showInformationMessage).mockResolvedValueOnce(undefined as any)
+    await runCLICommand({} as any, false, {} as any, {} as any)(cell)
+
+    expect(vi.mocked(terminal.sendText)).not.toHaveBeenCalled()
+
+    vi.mocked(terminal.sendText).mockClear()
+
+    // Saved
+    vi.mocked(window.showInformationMessage).mockResolvedValueOnce('Save' as any)
+    await runCLICommand({} as any, false, {} as any, {} as any)(cell)
+
+    expect(vi.mocked(terminal.sendText)).toHaveBeenCalled()
+    expect(cell.notebook.save).toHaveBeenCalledOnce()
+  })
 })
 
 test('open markdown as Runme notebook', (file: NotebookDocument) => {
diff --git a/tests/extension/configuration.test.ts b/tests/extension/configuration.test.ts
index 58220926..f1b1ec93 100644
--- a/tests/extension/configuration.test.ts
+++ b/tests/extension/configuration.test.ts
@@ -12,7 +12,8 @@ import {
   DEFAULT_TLS_DIR,
   getNotebookTerminalFontFamily,
   getNotebookTerminalFontSize,
-  getCodeLensEnabled
+  getCodeLensEnabled,
+  getCLIUseIntegratedRunme
 } from '../../src/utils/configuration'
 import { SERVER_PORT } from '../../src/constants'
 
@@ -150,6 +151,10 @@ suite('Configuration', () => {
       ).toStrictEqual(true)
     })
 
+    test('getCLIUseIntegratexRunme should return false by default', () => {
+      expect(getCLIUseIntegratedRunme()).toStrictEqual(false)
+    })
+
     suite('posix', () => {
       beforeEach(() => {
         platformPathMocks(path.posix)