Fix cell executor selection logic so bash cell execution works

[vezwork]

Fixes #687

There was a bug in the logic that checked if a cell executor required an extension to run. As a result, bash cells could not be run. That is fixed, and you can send bash/sh cells to the console now. Also added some logic to avoid sending bash code cells to terminals running R language interactive sessions.

[cscheid]

Am I missing something or should this ensure that the terminal is running bash? There's probably at least one sicko out there running powershell on macos as their main shell. (And will this work on windows where... isn't powershell is the default?)

[vezwork]

Am I missing something or should this ensure that the terminal is running bash? There's probably at least one sicko out there running powershell on macos as their main shell. (And will this work on windows where... isn't powershell is the default?)

You must be right? I am not sure how to check though. Will look into it, but I think we should proceed with the fix regardless? This is a fix of a regression after all.

[cscheid]

My comment on the PR thread itself was:

Am I missing something or should this ensure that the terminal is running bash? There's probably at least one sicko out there running powershell on macos as their main shell. (And will this work on windows where... isn't powershell is the default?)

From my read of the code, Elliot is correct that this PR doesn't add a new problem. But it might surface the underling problem more broadly. The previous logic might have caused the bash cells to be unavailable on Windows, and that might no longer be the case.

I really think we ought to test this on Windows...

[juliasilge]

This does fix the problem for me locally. ✅

I would be entirely unsurprised if this never worked at all on Windows, and from my experience running tests with this infrastructure on different OSes, it would take some very bespoke test infra setup to cover the Windows terminal options. Not unimportant, but also IMO not the highest priority right now compared to other options to spend time on.

[cscheid]

Sorry, I should have clarified - I didn't mean automated testing. I just meant "someone with a windows computer should try this fix and see what it does".

[juliasilge]

Can we get @cderv to try this perhaps? You can get the .vsix built from this branch at:

https://github.com/quarto-dev/quarto/actions/runs/17988978857/artifacts/4098060255

Download, unzip, then do Extensions: Install from VSIX...

[cderv]

As this is sending to terminal, it won't matter which is the terminal. So

ls

will send the command to the active terminal. In my case this is powershell. ls is a known alias for powershell equivalent, and so it works.

But if it were bash only command, then this would not.

To clarify, this implementation is different than running the cell through quarto, with engine knitr for example.

A bash chunk in a .qmd / .Rmd documents, requires on windows to have bash emulator (like Git bash) and configure knitr to use it. So usually a Windows user would not have bash chunk.

For Jupyter engine, I guess this would be the same.

Here is the working part when running cell, and error we see on Windows when rendering with Quarto

So this PR fixes the issue, of the initial feature that was introduced in

• Ctrl+Enter executes R and Python code but not bash #4

However, ultimately, this type of feature raises the question of the expectation for user between running interactively and rendering.

But you know the discussion...

Sorry I missed that PR - happy to help on windows stuff always!

[cscheid]

But if it were bash only command, then this would not.

I think this is what worries me. It's a "bash" cell. It should work with bash-only commands.

[cderv]

Currently, it is a convenient feature that assumes users running .qmd cell interactively have the same terminal as the terminal cell they are using... 😅

Not sure if there is a way to check the terminal that is open, and compare to the name of the cell language.

[vezwork]

I agree that it is somewhat problematic that the bash cell is executed in whatever terminal you have laying around (for clarity, cross-system consistency, reproducibility reasons). At the same time, it seems quite convenient and nice that it opens up/uses the terminal you have in your editor.

We can specify a custom shellPath when creating the terminal...

function window.createTerminal(name?: string | undefined, shellPath?: string | undefined, shellArgs?: string | readonly string[] | undefined): vscode.Terminal (+2 overloads)
Creates a Terminal with a backing shell process. The cwd of the terminal will be the workspace directory if it exists.

@param name — Optional human-readable string which will be used to represent the terminal in the UI.

@param shellPath — Optional path to a custom shell executable to be used in the terminal.

@param shellArgs
Optional args for the custom shell executable. A string can be used on Windows only which allows specifying shell args in command-line format.

@return — A new Terminal.

Here's a terminal object. We could check if the running terminal's name === 'bash' (or 'zsh'?)...

Seems like a bit of a rabbit hole... should I go there? Or should I, for now, let the user run the cell in their terminal, whatever it might be?

[juliasilge]

I would advocate for letting the user run the cell in their terminal for now (i.e. fix the regression), and looking for user feedback on specifics for when this isn't working, to inform future work.