Repository navigation
Run preview and render as tasks with ShellExecution, so VS Code quotes arguments for the shell #1203
Description
Summary
The extension runs Quarto for preview and render with terminal.sendText(). It builds the command line itself, so it must quote each argument for the shell that the terminal starts. In #1197, we are correcting some reported problems (#1194, #1102, posit-dev/positron#15907) by getting the shell from vscode.env.shell and quoting for bash, zsh, sh, PowerShell, and cmd. However, this approach still guesses which shell reads the command line.
I'd like to propose to run these commands as tasks with ShellExecution, and to give the arguments as ShellQuotedString values. Then the task system finds the shell and quotes each argument for that shell. Positron uses this approach, and it works well there.
Problems that remain with the current approach
- The extension knows only bash, zsh, sh, PowerShell, and cmd. For all other shells (for example fish, nu, and WSL), it quotes only on whitespace. As a result, paths with parentheses,
;, or$still break in these shells. For example, fish runs(md)as a command substitution. vscode.env.shellgives the default profile at the time that the extension sends the command. This shell is not always the shell that the terminal runs.- With
useCmd(Quarto 1.1.162 and earlier on Windows), the extension quotes the path for cmd.exe and puts the line incmd /C"...". But the extension types this line into the default shell of the user, which is often PowerShell. - The extension quotes only the document path. It does not quote the other arguments. For example, R package preview adds
--output-dir <temp dir>with no quotes, so a Windows user name with a space breaks the preview.
Scope of the change
Render (apps/vscode/src/providers/render.ts) would be the best place to start. It makes a terminal, sends the command, and removes the terminal by name before the next render. No other code tracks this terminal.
Preview (apps/vscode/src/providers/preview/preview.ts) needs more work, because its design uses a Terminal object:
- It keeps
this.terminal_, readsexitStatus, callsshow(), and usesonDidCloseTerminalto send the terminate request. - The Positron Viewer stop button (Show Positron's Viewer stop button for Quarto previews #1193) uses
onDidEndTerminalShellExecutionfor that terminal. After the user stops the preview, the terminal stays open, so the user can read the output. killTerminal()finds the terminal by its name.
A task gives a TaskExecution, not a Terminal. Thus this lifecycle must move to tasks.onDidStartTaskProcess, tasks.onDidEndTaskProcess, and TaskExecution.terminate().
Open questions
- Python environment activation: before it types the command,
sendTerminalCommand()waits up to 5 seconds for venv or conda activation. How does activation work in task terminals, in VS Code and in Positron? - After the user stops the preview, can a task terminal keep its output visible, as the stop button does now?
- Can we remove the
useCmdpath? It applies only to Quarto 1.1.162 and earlier.
Proposed plan
- Move render to
ShellExecution. Do a test with conda and venv environments. - Move preview, with the stop button and the restart behavior, in VS Code and in Positron.
- When no code uses them, remove the shell detection and quoting from
terminalCommand()(shQuote(),shQuoteUnknownShell(),terminalShellForPath()).
Activity