better populate env variables

examples/README.md

@@ -80,10 +80,7 @@ echo "SERVICE_BAR_TOKEN: $SERVICE_BAR_TOKEN"
 
 Supports changes to `$PATH`:
 
-```sh
-export PATH="~/.deno/bin:$PATH"
-deployctl deploy \
-    --project=runme-staging \
-    --import-map=import_map.json main.ts \
-    --token=$DENO_ACCESS_TOKEN
+```sh { interactive=false }
+export PATH="/some/path:$PATH"
+echo $PATH
 ```

package.json

@@ -118,6 +118,8 @@
     "@types/yargs": "^17.0.13",
     "@typescript-eslint/eslint-plugin": "^5.38.0",
     "@typescript-eslint/parser": "^5.38.0",
+    "@vitest/coverage-c8": "^0.23.4",
+    "c8": "^7.12.0",
     "copy-webpack-plugin": "^11.0.0",
     "eslint": "^8.24.0",
     "eslint-import-resolver-typescript": "^3.5.1",
@@ -131,6 +133,7 @@
     "ts-loader": "^9.4.1",
     "ts-node": "^10.9.1",
     "typescript": "^4.8.3",
+    "vitest": "^0.23.4",
     "vsce": "^2.11.0",
     "vscode-notebook-error-overlay": "^1.0.1",
     "webpack": "^5.74.0",

src/extension/executors/shell.ts

@@ -1,32 +1,18 @@
-import path from 'node:path'
-import { writeFile, chmod } from 'node:fs/promises'
 import { spawn } from 'node:child_process'
 
-import {
-  TextDocument, NotebookCellOutput, NotebookCellOutputItem, NotebookCellExecution,
-  ExtensionContext
-} from 'vscode'
-import { file } from 'tmp-promise'
+import { NotebookCellOutput, NotebookCellOutputItem, NotebookCellExecution } from 'vscode'
 
-import { OutputType, STATE_KEY_FOR_ENV_VARS } from '../../constants'
+import { OutputType } from '../../constants'
 import type { CellOutput } from '../../types'
 
 async function shellExecutor(
-  context: ExtensionContext,
   exec: NotebookCellExecution,
-  doc: TextDocument
+  scriptPath: string,
+  cwd: string,
+  env: Record<string, string>
 ): Promise<boolean> {
   const outputItems: string[] = []
-  const scriptFile = await file()
-  await writeFile(scriptFile.path, doc.getText(), 'utf-8')
-  await chmod(scriptFile.path, 0o775)
-
-  const stateEnv: Record<string, string> = context.globalState.get(STATE_KEY_FOR_ENV_VARS, {})
-  const child = spawn(scriptFile.path, {
-    cwd: path.dirname(doc.uri.path),
-    shell: true,
-    env: { ...process.env, ...stateEnv }
-  })
+  const child = spawn(scriptPath, { cwd, shell: true, env })
   console.log(`[RunMe] Started process on pid ${child.pid}`)
 
   /**

src/extension/executors/task.ts

@@ -11,6 +11,7 @@ import {
 } from 'vscode'
 import { file } from 'tmp-promise'
 
+import { populateEnvVar } from '../utils'
 import { STATE_KEY_FOR_ENV_VARS } from '../../constants'
 
 // import { ExperimentalTerminal } from "../terminal"
@@ -41,18 +42,17 @@ async function taskExecutor(
   const exportMatches = (doc.getText().match(EXPORT_REGEX) || [])
     .filter((m) => m.indexOf('export PATH=') <= 0)
     .map((m) => m.trim())
-  const rawEnv = Object.entries(context.workspaceState.get<Record<string, string>>(STATE_KEY_FOR_ENV_VARS, {}))
-  const stateEnv: Record<string, string> = Object.fromEntries(rawEnv.filter(([k]) => k !== 'PATH'))
+  const stateEnv = context.workspaceState.get<Record<string, string>>(STATE_KEY_FOR_ENV_VARS, {})
   for (const e of exportMatches) {
     const [key, ph] = e.slice('export '.length).split('=')
     const hasStringValue = ph.startsWith('"')
     const placeHolder = hasStringValue ? ph.slice(1) : ph
-    stateEnv[key] = await window.showInputBox({
+    stateEnv[key] = populateEnvVar(await window.showInputBox({
       title: `Set Environment Variable "${key}"`,
       placeHolder,
       prompt: 'Your shell script wants to set some environment variables, please enter them here.',
       ...(hasStringValue ? { value: placeHolder } : {})
-    }) || ''
+    }) || '', stateEnv)
 
     /**
      * we don't want to run these exports anymore as we already stored
@@ -70,48 +70,43 @@ async function taskExecutor(
   }
   await context.workspaceState.update(STATE_KEY_FOR_ENV_VARS, stateEnv)
 
-  /**
-   * run as non interactive shell script if set as configuration or annotated
-   * in markdown section
-   */
-  const config = workspace.getConfiguration('runme')
-  if (!config.get('shell.interactive') || exec.cell.metadata.attributes?.interactive === 'false') {
-    return inlineSh(context, exec, doc)
-  }
-
+  const cwd = path.dirname(doc.uri.path)
   const scriptFile = await file()
   const splits = scriptFile.path.split('-')
   const id = splits[splits.length-1]
   const RUNME_ID = `${doc.fileName}:${exec.cell.index}`
+  const env = {
+    ...process.env,
+    // eslint-disable-next-line @typescript-eslint/naming-convention
+    RUNME_TASK: 'true',
+    // eslint-disable-next-line @typescript-eslint/naming-convention
+    RUNME_ID,
+    ...stateEnv
+  }
 
   await writeFile(scriptFile.path, cellText, 'utf-8')
   await chmod(scriptFile.path, 0o775)
 
+  /**
+   * run as non interactive shell script if set as configuration or annotated
+   * in markdown section
+   */
+  const config = workspace.getConfiguration('runme')
+  if (!config.get('shell.interactive') || exec.cell.metadata.attributes?.interactive === 'false') {
+    return inlineSh(exec, scriptFile.path, cwd, env)
+  }
+
   const taskExecution = new Task(
     { type: 'runme', name: `Runme Task (${id})` },
     TaskScope.Workspace,
     cellText.length > LABEL_LIMIT
       ? `${cellText.slice(0, LABEL_LIMIT)}...`
       : cellText,
     'exec',
-    new ShellExecution(scriptFile.path, {
-      cwd: path.dirname(doc.uri.path),
-      env: {
-        ...process.env,
-        // eslint-disable-next-line @typescript-eslint/naming-convention
-        RUNME_TASK: 'true',
-        // eslint-disable-next-line @typescript-eslint/naming-convention
-        RUNME_ID,
-        ...stateEnv
-      }
-    }),
+    new ShellExecution(scriptFile.path, { cwd, env })
     // experimental only
     // new CustomExecution(async (): Promise<Pseudoterminal> => {
-    //   return new ExperimentalTerminal(scriptFile.path, {
-    //     cwd: path.dirname(doc.uri.path),
-    //     // eslint-disable-next-line @typescript-eslint/naming-convention
-    //     env: { RUNME_TASK: "true", RUNME_ID },
-    //   })
+    //   return new ExperimentalTerminal(scriptFile.path, { cwd, env })
     // })
   )
   const isBackground = exec.cell.metadata.attributes?.['background'] === 'true'

src/extension/utils.ts

@@ -1,26 +1,19 @@
 import vscode from 'vscode'
 
+const ENV_VAR_REGEXP = /(\$\w+)/g
+
 export function isInteractiveTask (cell: vscode.NotebookCell) {
   const config = vscode.workspace.getConfiguration('runme')
+  const configSetting = config.get<boolean>('shell.interactive', true)
 
   /**
    * if cell is marked as interactive (default: not set or set to 'true')
    */
-  if (
-    typeof cell.metadata.attributes?.interactive === 'undefined' ||
-    cell.metadata.attributes.interactive === 'true'
-  ) {
-    return true
-  }
-
-  /**
-   * if it is set within the settings (default: true)
-   */
-  if (config.get('shell.interactive')) {
+  if (cell.metadata?.attributes && cell.metadata.attributes.interactive === 'true') {
     return true
   }
 
-  return false
+  return configSetting
 }
 
 export function getTerminalByCell (cell: vscode.NotebookCell) {
@@ -29,3 +22,12 @@ export function getTerminalByCell (cell: vscode.NotebookCell) {
     return taskEnv.RUNME_ID === `${cell.document.fileName}:${cell.index}`
   })
 }
+
+export function populateEnvVar (value: string, env = process.env) {
+  for (const m of value.match(ENV_VAR_REGEXP) || []) {
+    const envVar = m.slice(1) // slice out '$'
+    value = value.replace(m, env[envVar] || '')
+  }
+
+  return value
+}

tests/extension/utilts.test.ts

@@ -0,0 +1,44 @@
+import vscode from 'vscode'
+import { expect, vi, test } from 'vitest'
+
+import { isInteractiveTask, getTerminalByCell, populateEnvVar } from '../../src/extension/utils'
+
+vi.mock('vscode', () => ({
+  default: {
+    window: {
+      terminals: [
+        { creationOptions: { env: {} } },
+        { creationOptions: { env: { RUNME_ID: 'foobar:123' } } }
+      ]
+    },
+    workspace: {
+      getConfiguration: vi.fn()
+    }
+  }
+}))
+
+test('isInteractive', () => {
+  // when set to false in configutaration
+  vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ get: vi.fn().mockReturnValue(false) } as any)
+  expect(isInteractiveTask({ metadata: {} } as any)).toBe(false)
+  expect(isInteractiveTask({ metadata: { attributes: {} } } as any)).toBe(false)
+  expect(isInteractiveTask({ metadata: { attributes: { interactive: 'true' } } } as any)).toBe(true)
+
+  vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ get: vi.fn().mockReturnValue(true) } as any)
+  expect(isInteractiveTask({ metadata: {} } as any)).toBe(true)
+  expect(isInteractiveTask({ metadata: { attributes: {} } } as any)).toBe(true)
+})
+
+test('getTerminalByCell', () => {
+  expect(getTerminalByCell({ document: { fileName: 'foo' }, index: 42} as any))
+    .toBe(undefined)
+  expect(getTerminalByCell({ document: { fileName: 'foobar' }, index: 123} as any))
+    .not.toBe(undefined)
+})
+
+test('populateEnvVar', () => {
+  expect(populateEnvVar(
+    'export PATH="/foo/$BAR/$LOO:$PATH:/$FOO"',
+    { PATH: '/usr/bin', FOO: 'foo', BAR: 'bar' }
+  )).toBe('export PATH="/foo/bar/:/usr/bin:/foo"')
+})

vitest.conf.ts

@@ -0,0 +1,23 @@
+/// <reference types="vitest" />
+import { defineConfig } from 'vitest/config'
+
+export default defineConfig({
+  test: {
+    include: ['test/**/*.test.ts'],
+    /**
+     * not to ESM ported packages
+     */
+    exclude: [
+      'dist', '.idea', '.git', '.cache',
+      '**/node_modules/**'
+    ],
+    coverage: {
+      enabled: false,
+      exclude: ['**/build/**', '**/__fixtures__/**', '**/*.test.ts'],
+      lines: 100,
+      functions: 100,
+      branches: 100,
+      statements: 100
+    }
+  }
+})