Skip to content

Commit

Permalink
Prepare the rmarkdown::render() code snippet for the challenge of P…
Browse files Browse the repository at this point in the history
…owerShell (#3909)

Addresses #3816 by using `vscode.ProcessExecution` instead of
`vscode.ShellExecution`, to bypass some tricky quoting issues presented
by PowerShell, without doing our own quoting or writing (much) OS- or
shell-specific code.

The problem is that we want to execute a code snippet like
`rmarkdown::render("whatever.Rmd")` (where `"whatever.Rmd"` needs to be
interpolated in, which is really neither here nor there). So you need
quotes around this file path. But using `ShellExecution` you also need
quotes around the whole `rmarkdown::render()` snippet; let's say these
are single quotes. And, when executed in PowerShell, the entire command
_also_ gets double quoted, which then changes the context for the quotes
in the code snippet, leading to much misery:

```
The terminal process "C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe -Command &
'C:\Program Files\R\R-4.3.3\bin\x64\R.exe' -e 'rmarkdown::render("d:\Users\jenny\rmd-render-
fun\test.Rmd")'" terminated with exit code: 1. 
```

By the time you get to R, the double quotes around the file path have
gone missing (since I captured this error, I have changed the path
separators here as well; but also that is not the problem):

```
> rmarkdown::render(d:\Users\jenny\rmd-render-fun\test.Rmd)
Error: unexpected symbol in "rmarkdown::render(d:\Users"
Execution halted
```

I tried all sorts of things, trying to stick with
`vscode.ShellExecution` and `vscode.ShellQuoting` but never succeeded in
finding a combination of argument strings and quoting that worked on
Windows (PowerShell) and macOS. I became increasingly convinced, similar
to this issue microsoft/vscode#187661, that it
might be impossible.

### QA Notes

Exercise these commands on multiple OSes:

* *R: Render Document with R Markdown*, find this in the command palette
* *R: Source R File*, find this in the command palette or use the "play"
button

Here's a repo I used for experimentation while working on this PR:
https://github.com/jennybc/rmd-render-fun.

You could grab it with

```r
usethis::create_from_github("https://github.com/jennybc/rmd-render-fun", destdir = "~/tmp")
```

Substitute your favorite destination directory for experiments in
`destdir`. This folder sports 2 `.Rmd` files, with and without spaces in
the filename, and likewise for `.R`.
  • Loading branch information
jennybc authored Jul 17, 2024
1 parent 02fb767 commit b189a8b
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 23 deletions.
14 changes: 6 additions & 8 deletions extensions/positron-r/src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,10 @@ export async function registerCommands(context: vscode.ExtensionContext) {
vscode.commands.registerCommand('r.sourceCurrentFile', async () => {
try {
const filePath = await getEditorFilePathForCommand();
// In the future, we may want to shorten the path by making it
// relative to the current working directory.
if (filePath) {
const command = `source(${filePath})`;
const command = `source(${JSON.stringify(filePath)})`;
positron.runtime.executeCode('r', command, false);
}
} catch (e) {
Expand Down Expand Up @@ -324,14 +326,10 @@ export async function getEditorFilePathForCommand() {
// the VS Code file system API.
const fsStat = await vscode.workspace.fs.stat(vscode.Uri.file(filePath));

// In the future, we will want to shorten the path by making it
// relative to the current directory; doing so, however, will
// require the kernel to alert us to the current working directory.
//
// For now, just use the full path, passed through JSON encoding
// to ensure that it is properly escaped.
// Return the full path, with POSIX path separators. Any additional path
// math, escaping, or quoting is the responsibility of the caller.
if (fsStat) {
return JSON.stringify(filePath);
return filePath.replace(/\\/g, '/');
}
return;
}
50 changes: 35 additions & 15 deletions extensions/positron-r/src/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
*--------------------------------------------------------------------------------------------*/

import * as vscode from 'vscode';
import * as os from 'os';
import { RSessionManager } from './session-manager';
import { getEditorFilePathForCommand } from './commands';
import { getPandocPath } from './pandoc';

export class RPackageTaskProvider implements vscode.TaskProvider {
Expand Down Expand Up @@ -54,7 +54,7 @@ export async function getRPackageTasks(editorFilePath?: string): Promise<vscode.
{
task: 'r.task.rmarkdownRender',
message: vscode.l10n.t('{taskName}', { taskName: 'Render document with R Markdown' }),
rcode: `rmarkdown::render(${editorFilePath})`,
rcode: `rmarkdown::render("${editorFilePath}")`,
package: 'rmarkdown'
}
];
Expand All @@ -67,17 +67,37 @@ export async function getRPackageTasks(editorFilePath?: string): Promise<vscode.
env['RSTUDIO_PANDOC'] = pandocPath;
}

// the explicit quoting treatment is necessary to avoid headaches on Windows, with PowerShell
return taskData.map(data => new vscode.Task(
{ type: 'rPackageTask', task: data.task, pkg: data.package },
vscode.TaskScope.Workspace,
data.message,
'R',
new vscode.ShellExecution(
binpath,
['-e', { value: data.rcode, quoting: vscode.ShellQuoting.Strong }],
{ env }
),
[]
));
return taskData.map(data => {
let exec: vscode.ProcessExecution | vscode.ShellExecution;
if (data.task === 'r.task.rmarkdownRender' && os.platform() === 'win32') {
// Using vscode.ProcessExecution gets around some hairy quoting issues on Windows,
// specifically encountered with PowerShell.
// https://github.com/posit-dev/positron/issues/3816
// We don't know of specific problems around not using a shell (for example, env vars
// appear to be inherited by ProcessExecution), but we're still scoping this narrowly
// out of caution.
exec = new vscode.ProcessExecution(
binpath,
['-e', data.rcode],
{ env }
);
} else {
// The explicit quoting treatment here is also motivated by PowerShell, so make sure to
// test any changes on Windows.
exec = new vscode.ShellExecution(
binpath,
['-e', { value: data.rcode, quoting: vscode.ShellQuoting.Strong }],
{ env }
);
}

return new vscode.Task(
{ type: 'rPackageTask', task: data.task, pkg: data.package },
vscode.TaskScope.Workspace,
data.message,
'R',
exec,
[]
);
});
}

0 comments on commit b189a8b

Please sign in to comment.