Repository: stateful/vscode-runme
Pull request #335 (2023-03-24 19:11:59)

Integrate CLI button with gRPC runner
This fixes the CLI button for use with the gRPC runner. Needs version of binary including stateful/runme#202 (built from main)

Not sure whether to pass everything as command line parameters, or environment variables. I used environment variables because it looks cleaner (and allows users to easily run other commands in the same session), but a lot is then "hidden" from the user. Thoughts @sourishkrout ?

sourishkrout: > Not sure whether to pass everything as command line parameters, or environment variables. I used environment variables because it looks cleaner (and allows users to easily run other commands in the same session), but a lot is then "hidden" from the user. Thoughts @sourishkrout ?

I was inclined to do the same. Mainly because with further explanation the flags aren't particularly self-evident and in this integrated way it makes sense for it to "just work".

Patch:
diff --git a/src/extension/commands/index.ts b/src/extension/commands/index.ts
index 547df1e6..90c7efd0 100644
--- a/src/extension/commands/index.ts
+++ b/src/extension/commands/index.ts
@@ -1,13 +1,17 @@
 import path from 'node:path'
+import os from 'node:os'
 
 import {
   NotebookCell, Uri, window, env, NotebookDocument, TextDocument, ViewColumn,
   workspace, NotebookData, commands, NotebookCellData, NotebookCellKind
 } from 'vscode'
 
+import { getBinaryPath, getTLSDir, getTLSEnabled } from '../../utils/configuration'
 import { Kernel } from '../kernel'
 import { CliProvider } from '../provider/cli'
 import { getAnnotations, getTerminalByCell } from '../utils'
+import RunmeServer from '../server/runmeServer'
+import { GrpcRunnerEnvironment } from '../runner'
 
 function showWarningMessage () {
   return window.showWarningMessage('Couldn\'t find terminal! Was it already closed?')
@@ -35,23 +39,58 @@ export function stopBackgroundTask (cell: NotebookCell) {
   return window.showInformationMessage(`${terminal?.name} task terminated!`)
 }
 
-export async function runCLICommand (cell: NotebookCell) {
-  if (!await CliProvider.isCliInstalled()) {
-    return window.showInformationMessage(
-      'Runme CLI is not installed. Do you want to download it?',
-      'Download now'
-    ).then((openBrowser) => openBrowser && env.openExternal(
-      Uri.parse('https://github.com/stateful/runme/releases')
-    ))
+export function runCLICommand(
+  extensionBaseUri: Uri,
+  grpcRunner: boolean,
+  server: RunmeServer,
+  kernel: Kernel
+) {
+ return async function(cell: NotebookCell) {
+    let runmeExecutable: string
+
+    const args = [
+      `--chdir="${path.dirname(cell.document.uri.fsPath)}"`,
+      `--filename="${path.basename(cell.document.uri.fsPath)}"`
+    ]
+
+    const envs: Record<string, string> = { }
+
+    if (grpcRunner) {
+      runmeExecutable = getBinaryPath(extensionBaseUri, os.platform()).fsPath
+
+      envs['RUNME_SERVER_ADDR'] = server.address()
+
+      if (getTLSEnabled()) {
+        envs['RUNME_TLS_DIR'] = getTLSDir()
+      } else {
+        args.push('--insecure')
+      }
+
+      const runnerEnv = kernel.getRunnerEnvironment()
+      if(runnerEnv && runnerEnv instanceof GrpcRunnerEnvironment) {
+        envs['RUNME_SESSION'] = runnerEnv.getSessionId()
+      }
+    } else {
+      if (!await CliProvider.isCliInstalled()) {
+        return window.showInformationMessage(
+          'Runme CLI is not installed. Do you want to download it?',
+          'Download now'
+        ).then((openBrowser) => openBrowser && env.openExternal(
+          Uri.parse('https://github.com/stateful/runme/releases')
+        ))
+      }
+
+      runmeExecutable = 'runme'
+    }
+
+    const annotations = getAnnotations(cell)
+    const term = window.createTerminal({
+      name: `CLI: ${annotations.name}`,
+      env: envs,
+    })
+    term.show(false)
+    term.sendText(`${runmeExecutable} run ${annotations.name} ${args.join(' ')}`)
   }
-  const annotations = getAnnotations(cell)
-  const term = window.createTerminal(`CLI: ${annotations.name}`)
-  const args = [
-    `--chdir="${path.dirname(cell.document.uri.fsPath)}"`,
-    `--filename="${path.basename(cell.document.uri.fsPath)}"`
-  ]
-  term.show(false)
-  term.sendText(`runme run ${annotations.name} ${args.join(' ')}`)
 }
 
 export function openAsRunmeNotebook (doc: NotebookDocument) {
diff --git a/src/extension/extension.ts b/src/extension/extension.ts
index 0a55b617..87f083ae 100644
--- a/src/extension/extension.ts
+++ b/src/extension/extension.ts
@@ -85,7 +85,10 @@ export class RunmeExtension {
 
       commands.registerCommand('runme.resetEnv', resetEnv),
       RunmeExtension.registerCommand('runme.openTerminal', openTerminal),
-      RunmeExtension.registerCommand('runme.runCliCommand', runCLICommand),
+      RunmeExtension.registerCommand(
+        'runme.runCliCommand',
+        runCLICommand(context.extensionUri, !!grpcRunner, server, kernel)
+      ),
       RunmeExtension.registerCommand('runme.copyCellToClipboard', copyCellToClipboard),
       RunmeExtension.registerCommand('runme.stopBackgroundTask', stopBackgroundTask),
       RunmeExtension.registerCommand('runme.openSplitViewAsMarkdownText', openSplitViewAsMarkdownText),
diff --git a/src/extension/server/runmeServer.ts b/src/extension/server/runmeServer.ts
index 15ea22a6..47bf087f 100644
--- a/src/extension/server/runmeServer.ts
+++ b/src/extension/server/runmeServer.ts
@@ -90,7 +90,7 @@ class RunmeServer implements Disposable {
       }
     }
 
-    protected address() {
+    address() {
       return `${SERVER_ADDRESS}:${this.#port}`
     }
 


Test patch:
diff --git a/tests/extension/commands/index.test.ts b/tests/extension/commands/index.test.ts
index c60622ed..32d0026e 100644
--- a/tests/extension/commands/index.test.ts
+++ b/tests/extension/commands/index.test.ts
@@ -38,6 +38,14 @@ vi.mock('../../../src/extension/provider/cli', () => ({
   }
 }))
 
+vi.mock('../../../src/extension/server/runmeServer.ts', () => ({
+  default: class { }
+}))
+
+vi.mock('../../../src/extension/runner', () => ({
+  GrpcRunnerEnvironment: class { }
+}))
+
 beforeEach(() => {
   vi.mocked(window.showWarningMessage).mockClear()
   vi.mocked(window.showInformationMessage).mockClear()
@@ -60,12 +68,15 @@ test('copyCellToClipboard', () => {
 })
 
 test('runCLICommand if CLI is not installed', async () => {
-  const cell: any = {}
+  const cell: any = {
+    metadata: { name: 'foobar' },
+    document: { uri: { fsPath: '/foo/bar/README.md' }}
+  }
   vi.mocked(window.showInformationMessage).mockResolvedValueOnce(false as any)
-  await runCLICommand(cell)
+  await runCLICommand({} as any, false, {} as any, {} as any)(cell)
   expect(env.openExternal).toBeCalledTimes(0)
   vi.mocked(window.showInformationMessage).mockResolvedValue(true as any)
-  await runCLICommand(cell)
+  await runCLICommand({} as any, false, {} as any, {} as any)(cell)
   expect(env.openExternal).toBeCalledTimes(1)
 })
 
@@ -76,8 +87,11 @@ test('runCLICommand if CLI is installed', async () => {
   }
   vi.mocked(CliProvider.isCliInstalled).mockResolvedValue(true)
   vi.mocked(getAnnotations).mockReturnValue(cell.metadata)
-  await runCLICommand(cell)
-  expect(window.createTerminal).toBeCalledWith('CLI: foobar')
+  await runCLICommand({} as any, false, {} as any, {} as any)(cell)
+  expect(window.createTerminal).toBeCalledWith({
+    env: {},
+    name: 'CLI: foobar'
+  })
   expect(terminal.show).toBeCalledTimes(1)
   expect(terminal.sendText).toBeCalledWith('runme run foobar --chdir="/foo/bar" --filename="README.md"')
 })