Skip to content

Add runSelectionInActiveTerm command #80 #102#104

Merged
Ikuyadeu merged 6 commits intoREditorSupport:masterfrom
andycraig:issue-80-send-to-current-terminal
Apr 15, 2019
Merged

Add runSelectionInActiveTerm command #80 #102#104
Ikuyadeu merged 6 commits intoREditorSupport:masterfrom
andycraig:issue-80-send-to-current-terminal

Conversation

@andycraig
Copy link
Copy Markdown
Collaborator

Fix #80

Fix #102

This PR adds a new command r.runSelectionInActiveTerm, accessed from the command palette as R: Run Selection/Line in Active Terminal. It functions like r.runSelection, but sends the selection to the currently active terminal instead of the terminal registered as rTerm.

What problem did you solve?

Can now send code to an R session not started via vscode-R (e.g., an R session started in an SSH session).

Can now run multiple R terminals at the same time and send code to the currently active terminal.

Screenshot

PR80

Notes

This PR increases the required VSCode engine to 1.29.0, to access vscode.window.activeTerminal.

There is no check for whether there is actually an R session running in the active terminal. The code will be sent regardless.

The new command is not bound to any keyboard shortcut by default.

An equivalent command is added for r.runSelection only. Other commands that send code (r.nrow, r.length, r.head, r.thead, r.names, r.runSource) still send code to rTerm only. If there is demand for further equivalent commands, they could be added in a future PR.

@Ikuyadeu
Copy link
Copy Markdown
Member

Ikuyadeu commented Apr 4, 2019

@andycraig Great work! Your PR information makes me easily understand to contents.
I will check this PR at the weekend.

@Ikuyadeu Ikuyadeu self-requested a review April 4, 2019 01:43
@Ikuyadeu
Copy link
Copy Markdown
Member

Ikuyadeu commented Apr 6, 2019

@andycraig OK, I checked. Great work.

I fixed the default runSelection behavior a bit.
Currently, the user needs to call a command.
But it wastes time as same as copy and pastes the source code.

In previous:

  1. You make R terminal manually (And the terminal name is R or R interactive)
  2. You call command from F1 key

Now:

  1. You make R terminal manually (And the terminal name is R or R interactive)
  2. You call just runSelection. If the terminal name is R or R interactive, runSelection command will choose that.

I will merge if it will fit for your image.

@Ikuyadeu
Copy link
Copy Markdown
Member

Ikuyadeu commented Apr 6, 2019

Screen Recording 2019-04-06 at 15 33 39
Here is movie, I just put cmd + Enter key 3 times.

Comment thread README.md
![Create R terminal](images/terminal.png)

* Run code in terminal containing existing R session, for example over SSH (`Run Selection/Line in Active Terminal`)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SSH support is awesome.
Do you have an example behavior of that?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ikuyadeu I added a movie showing use of R in an SSH session to the README.

@andycraig
Copy link
Copy Markdown
Collaborator Author

@Ikuyadeu Thank you for having a look at this PR and streamlining it! I think using runSelection for multiple 'R'/'R Interactive' terminals is better than my approach.

When using ssh from the command line and then running R, the terminal name is 'ssh', not 'R'. So to deal with 'ssh' terminals you would still need to use runSelectionInActiveTerm (which a frequent user would probably assign to a keyboard shortcut).

Your changes look good to me. Later today, I can add a movie showing use with SSH to the README, and if it looks okay to you I think it will be ready for merging.

@andycraig
Copy link
Copy Markdown
Collaborator Author

@Ikuyadeu I just noticed that now if there are no open terminals and the user tries to use runSelection, they get an error message 'There are no open terminals', but I think the preferred behaviour now would be for an R Interactive terminal to be created. I will add a commit for this.

@Ikuyadeu
Copy link
Copy Markdown
Member

Ikuyadeu commented Apr 7, 2019

@andycraig Thank you for your check.
I am looking forward to that, but please do not too hard.

@andycraig
Copy link
Copy Markdown
Collaborator Author

Thank you, @Ikuyadeu. I wasn't able to spend much more time on it today so I will carry on with it this week.

@andycraig
Copy link
Copy Markdown
Collaborator Author

It seems that creating and showing a terminal does not necessarily set activeTerminal, which is making this more complicated than it seems like it should be. From microsoft/vscode#64390, 'show is an async request which may or may not change the activeTerminal', and it sounds like onDidChangeActiveTerminal might be needed.

I will continue to work into this.

@andycraig
Copy link
Copy Markdown
Collaborator Author

I didn't have any success using a listener (e.g., onDidOpenTerminal), so I have gone with a workaround approach that seems to solve the problem.

@andycraig
Copy link
Copy Markdown
Collaborator Author

I'm going to make some improvements to that last commit, and then add a movie demonstrating SSH.

@andycraig
Copy link
Copy Markdown
Collaborator Author

@Ikuyadeu If everything looks okay to you, I am happy for this to be merged.

@Ikuyadeu Ikuyadeu merged commit ada6d52 into REditorSupport:master Apr 15, 2019
@Ikuyadeu
Copy link
Copy Markdown
Member

@andycraig Great works! I will publish a new version today.

@Ikuyadeu
Copy link
Copy Markdown
Member

I published a new one, also I added collaboraters (@andycraig and @Ladvien) on the README.
From the next version, I skip adding your name on CHANGELOG for separate other contributors.

@andycraig
Copy link
Copy Markdown
Collaborator Author

@Ikuyadeu I’m fine with my name being skipped in the CHANGELOG since I’m now mentioned in the README. Up to you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

R Terminal loses purchase of old terminals Only support sending code to local R Interactive

2 participants