Repository: stateful/vscode-runme
Pull request #146 (2023-01-25 14:33:13)

Mimic POSIX var substitution behavior
This will change the behavior of using `$(...)` expressions inside of exported ENV vars `export MY_VAR=$(echo 'test')`.

Old behavior inside of Runme notebook; notice gap (it's a `\n` but displayed as space by the markdown renderer) inside of MY_VAR:
```sh
$ export MY_CWD="$(pwd)"
$ export MY_VAR="$MY_CWD:$PATH"
$ echo $MY_VAR
/Users/sourishkrout/Projects/stateful/oss/runme :/Users/sourishkrout/.wasmtime/bin:/opt/homebrew/opt/libpq/bin:/Users/sourishkrout/go/bin:/Users/sourishkrout/.asdf/shims:/opt/homebrew/opt/asdf/libexec/bin:/Users/sourishkrout/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/Library/Apple/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Users/sourishkrout/.cache/zsh4humans/v5/fzf/bin
```

New behavior; no special endings (e.g. '\n'):
```sh
$ export MY_CWD="$(pwd)"
$ export MY_VAR="$MY_CWD:$PATH"
$ echo $MY_VAR
/Users/sourishkrout/Projects/stateful/oss/runme:/Users/sourishkrout/.wasmtime/bin:/opt/homebrew/opt/libpq/bin:/Users/sourishkrout/go/bin:/Users/sourishkrout/.asdf/shims:/opt/homebrew/opt/asdf/libexec/bin:/Users/sourishkrout/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/Library/Apple/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Users/sourishkrout/.cache/zsh4humans/v5/fzf/bin
```

Some tools are more tolerant than others. E.g. the PATH is sensitive and encoded strings passed into tools like `grpcurl` will fail as a result. Others are more fault-tolerant where they likely sanitize/trim previously extrapolated vars taken as input.

Whether or not this is exactly mimicking POSIX behavior it's far "less wrong" than the old behavior. Leaned on explanation from https://unix.stackexchange.com/questions/164508/why-do-newline-characters-get-lost-when-using-command-substitution/164548#164548 and specifically https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_03

This PR will also fix a bug where future var substitutions would not retain previously evaluated vars.


Patch:
diff --git a/package-lock.json b/package-lock.json
index 503e59cb..ad11a40e 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,12 +1,12 @@
 {
   "name": "runme",
-  "version": "0.5.0",
+  "version": "0.5.0-edge.3",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {
     "": {
       "name": "runme",
-      "version": "0.5.0",
+      "version": "0.5.0-edge.3",
       "hasInstallScript": true,
       "license": "Apache-2.0",
       "dependencies": {
diff --git a/package.json b/package.json
index a92fdc3c..4bc4d2f0 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "runme",
-  "version": "0.5.0",
+  "version": "0.5.0-edge.3",
   "publisher": "stateful",
   "displayName": "Runme",
   "description": "Run your README.md as an interactive notebook",
diff --git a/src/extension/executors/utils.ts b/src/extension/executors/utils.ts
index 78879f74..5e542892 100644
--- a/src/extension/executors/utils.ts
+++ b/src/extension/executors/utils.ts
@@ -62,6 +62,7 @@ export async function retrieveShellCommand (exec: NotebookCellExecution) {
       const expression = placeHolder.slice(2, -1)
       const expressionProcess = cp.spawn(expression, {
         cwd,
+        env: {...process.env, ...stateEnv },
         shell: true
       })
       const [isError, data] = await new Promise<[number, string]>((resolve) => {
@@ -69,6 +70,7 @@ export async function retrieveShellCommand (exec: NotebookCellExecution) {
         expressionProcess.stdout.on('data', (payload) => { data += payload.toString() })
         expressionProcess.stderr.on('data', (payload) => { data += payload.toString() })
         expressionProcess.on('close', (code) => {
+          data = data.trim()
           if (code && code > 0) {
             return resolve([code, data])
           }


Test patch:
diff --git a/tests/extension/executors/utils.test.ts b/tests/extension/executors/utils.test.ts
index b1ff28e5..60ba4bc4 100644
--- a/tests/extension/executors/utils.test.ts
+++ b/tests/extension/executors/utils.test.ts
@@ -121,7 +121,35 @@ test('can support expressions', async () => {
   process.env.BAR = 'bar'
   const cellText = await retrieveShellCommand(exec)
   expect(cellText).toBe('')
-  expect(ENV_STORE.get('foo')).toBe('foobar\n')
+  expect(ENV_STORE.get('foo')).toBe('foobar')
+})
+
+test('can recall previous vars', async () => {
+  const exec1: any = {
+    cell: {
+      metadata: { executeableCode: 'export foo=$(echo "foo$BAR")' },
+      document: {
+        getText: vi.fn().mockReturnValue('export foo=$(echo "foo$BAR")'),
+        uri: { fsPath: __dirname }
+      }
+    }
+  }
+  process.env.BAR = 'bar'
+  const cellText1 = await retrieveShellCommand(exec1)
+  expect(cellText1).toBe('')
+  expect(ENV_STORE.get('foo')).toBe('foobar')
+  const exec2: any = {
+    cell: {
+      metadata: { executeableCode: 'export barfoo=$(echo "bar$foo")' },
+      document: {
+        getText: vi.fn().mockReturnValue('export barfoo=$(echo "bar$foo")'),
+        uri: { fsPath: __dirname }
+      }
+    }
+  }
+  const cellText2 = await retrieveShellCommand(exec2)
+  expect(cellText2).toBe('')
+  expect(ENV_STORE.get('barfoo')).toBe('barfoobar')
 })
 
 test('returns undefined if expression fails', async () => {