handling terminals created by vscode-Python

[tomasnobrega]

What problem did you solve?

If Python extension and R extension are open at the same time on VSCode a conflict emerges because Python extension creates a new terminal.

vscode-R currently checks for only one terminal available at (vscode.window.terminals.length === 1). I propose to fix that by actively excluding the "Deactive" terminal.

This issue has been reported before:
#1459 (comment)
Note all 3 screenshots the second terminal is "Python ... Deactive" indicating the same root cause.

(If you do not have screenshot) How can I check this pull request?

To recreate the bug you should install Python VSCode extension and run some Python on VSCode (anything, just to activate the extension). Then close all R terminals and attempt to run a new R line of R code by using "Ctl + Enter" or "Cmd + Enter", should run into the error identifying terminal.

[andycraig]

Hi @tomasnobrega, thank you for submitting this PR and very nice work tracking down the cause of the error!

Looking at the screenshot, it appears that the Python extension may have recently started to use the Terminal property hideFromUser to create hidden terminals. I had a look at the hideFromUser docs, but it was a bit unclear to me what happens if one of those terminals becomes visible - maybe hideFromUser changes to false, or maybe it stays as true since it seems to be part of creationOptions. Since people only seem to be running into this problem while using the Python extension, I'm happy to keep the fix specific to the Python extension's hidden terminals at the moment. We can try to generalise it to all hidden terminals in the future if people still run into problems.

I'm going to leave a couple of comments on the code. After those are resolved, I'll do some manual testing. (At that point I might also need to reach out to one of the more active maintainers for help on resolving the failed CI checks.)

[tomasnobrega]

Hi Andy thank you for the helpful comments, I will make the changes over the weekend

[tomasnobrega]

@andycraig I just commited with requested changes. In my VS Code its working. Let me know if you need anything else

[andycraig]

@tomasnobrega Thank you very much for making these changes. I think it's fine to merge now now. I have written some notes below about some manual testing that I did, which turned up an edge case, but I think we can ignore it at present.

Details of manual tests I ran

With a file temp.R containing the code print("hello from R").
With the Python extension installed and a file temp.py containing the code print("hello from Python")

Setting r.alwaysUseActiveTerminal is false (the default):

Situation: There are 3 R terminals. The current terminal is the 2nd R terminal.

1. Create 3 terminals. Start R in each one. Select the 2nd terminal
2. With the cursor in temp.R, run R: Run Selection/Line
3. Successful outcome: Code is run in the CURRENT (2nd) R terminal

✅ PASSED

Situation: There are multiple R terminals. The current terminal is not an R terminal.

1. Create 2 terminals. Start R in the 1st and 2nd terminals. Select the last (non-R) terminal
2. With the cursor in temp.R, run R: Run Selection/Line
3. Successful outcome: Code is run in the LAST (2nd) R terminal.

✅ PASSED

Situation: Works when there is only one terminal, it is an R terminal, and it has just been created. (This did not work at one point due to a bug in VS Code.)

1. Kill the terminal that appears by default, using the trash icon
2. Create a terminal using R: Create R terminal
3. With the cursor in temp.R, run R: Run Selection/Line. (Do not do anything at all between steps 1. and 2., as that may affect what is considered the active terminal)
4. Successful outcome: Code is run in the R terminal

✅ PASSED

Situation: There WAS a Python terminal, but it has been closed. There is an R terminal. (This did not work at one point, probably due to a bug in VS Code or the Python extension.)

1. Create an R terminal with R: Create R terminal
2. With the cursor in temp.py, run Python: Run Selection/Line in Python terminal to create a Python terminal
3. Kill the Python terminal using the trash icon
4. With the cursor in temp.R, run R: Run Selection/Line
5. Successful outcome: Code is run in the R terminal

✅ PASSED

Setting r.alwaysUseActiveTerminal is true:

Situation: There are no terminals.

1. With the cursor in temp.R, run R: Run Selection/Line
2. OUTCOME: VS Code shows message 'There are no open terminals.'

✅ PASSED

Situation: There is an open terminal. Code is run in that terminal.

1. Create a non-R terminal with Terminal: Create New Terminal
2. With the cursor in temp.R, run R: Run Selection/Line
3. Successful outcome: Code is run in the terminal. (It will produce a bash error since R is not open in the terminal. That is fine)

✅ PASSED

Situation: There WAS a Python terminal, but it has been closed. There are no open terminals. (This did not work at one point, probably due to a bug in VS Code or the Python extension.)

1. With the cursor in temp.py, run Python: Run Selection/Line in Python terminal to create a Python terminal
2. Kill the Python terminal using the trash icon
3. With the cursor in temp.R, run R: Run Selection/Line
4. Successful outcome: VS Code shows message 'There are no open terminals.'

❌ FAILED (no VS Code message was shown)

The tests all passed except the last one, for the situation where r.alwaysUseActiveTerminal is true, a Python terminal has been created and closed, there are now no open terminals, and the user tries to send code to the (non-existent) terminal. In this case, VS Code should display a message to the user saying that there are no open terminals, but there is no message. I had a look, and in this case there were two hidden terminals: the Python terminal (which is detected and successfully ignored by the check for 'Deactivate' in the terminal name), and also a second hidden terminal just called 'bash'. So the 'bash' terminal is being counted as a terminal. Then chooseTerminal() runs return vscode.window.activeTerminal, which is undefined. In all locations where chooseTerminal() is called it checks whether the result is undefined and if so does nothing. In summary: (i) In this test case, the expected message is not displayed to the user, but it should be fairly clear to the user anyway that there is no (visible) terminal to send code to; (ii) Code is not run in a hidden terminal, which is good; and (iii) This is the current behaviour anyway, as the existing code also fails to handle the hidden terminals.

In future it would be nice to handle that edge case too, but this PR solves a problem that many users are experiencing and I am happy for it to be merged in its current form.

[andycraig]

@renkun-ken @ElianHugh @ManuelHentschel
Hi all, one of the CI checks is failing (macos-latest) and another was cancelled (windows-latest) but I don't know what needs to be done about them, if anything. Could you advise? If they are fine as-is, please go ahead and merge this PR as I am happy with the code itself.

[ElianHugh]

@andycraig I'm fairly certain the stops are just issues with the runner, it started happening recently and tends to resolve (at least for me) if the tests are reran until they succeed. Probably worth investigating, but I don't believe it has anything to do with this PR

Edit:

looks like the runner is deprecated: https://github.com/GabrielBB/xvfb-action

and the macos error mirrors the one linked here: microsoft/vscode#200708

[eitsupi]

Perhaps a node.js version issue?
It seems that a very old node.js 16 is currently specified and obviously needs to be updated.

[ElianHugh]

LGTM, thank you!

@eitsupi I'll create a new issue to track this, I'm not 100% familiar with the github actions used in this repo