Skip to content

Thread execute argument through to term.sendText()#585

Merged
andycraig merged 7 commits intoREditorSupport:masterfrom
krlmlr:f-dont-send
Mar 23, 2021
Merged

Thread execute argument through to term.sendText()#585
andycraig merged 7 commits intoREditorSupport:masterfrom
krlmlr:f-dont-send

Conversation

@krlmlr
Copy link
Copy Markdown
Contributor

@krlmlr krlmlr commented Mar 21, 2021

Closes #575.

I have no Idea if this code works, can't build: seeing Error: Cannot find module 'copy-webpack-plugin' when pressing F5 or Ctrl + Shift + B in VS Code.

There must be a better way to detect the last item in an iteration.

This code now also avoids the delay before the first line in non-bracketed paste.

@andycraig
Copy link
Copy Markdown
Collaborator

Thanks for the PR! Can you try running this at the command line before pressing F5? That will hopefully fix the error.

# In directory vscode-R
npm install

@krlmlr

This comment has been minimized.

@krlmlr

This comment has been minimized.

@krlmlr
Copy link
Copy Markdown
Contributor Author

krlmlr commented Mar 21, 2021

Works for me now.

Unrelated: I think I have made a mess on my system with various runs of apt install node-... and sudo npm install -g calls. Ideally I'd like to have a clean per-project setup, without systemwide installations. What's a good reference to get started?

@krlmlr
Copy link
Copy Markdown
Contributor Author

krlmlr commented Mar 21, 2021

I forgot to mention: the console message changed, "inserting" is shown with execute = FALSE, "sending" is now the default.

@krlmlr
Copy link
Copy Markdown
Contributor Author

krlmlr commented Mar 21, 2021

Done, tests succeed now. What's a good location for a test of this functionality?

@andycraig
Copy link
Copy Markdown
Collaborator

We don't have any unit tests for these functions that call term.sendText(), so I don't think it's necessary to add any tests as part of this PR.

Sounds like it's ready for review?

@krlmlr
Copy link
Copy Markdown
Contributor Author

krlmlr commented Mar 21, 2021

Yes, the implementation works for me.

Copy link
Copy Markdown
Collaborator

@andycraig andycraig left a comment

Choose a reason for hiding this comment

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

I've added a couple of minor notes, and everything else looks good.

I think you're using this with {fledge}, right? Could you write down the steps I can follow to cause it to send some text to the console without executing it? I'll follow those steps to test this PR. Thanks!

Comment thread src/rTerminal.ts Outdated
Comment thread src/rTerminal.ts Outdated
krlmlr and others added 3 commits March 23, 2021 04:25
@krlmlr
Copy link
Copy Markdown
Contributor Author

krlmlr commented Mar 23, 2021

I test with:

rstudioapi::sendToConsole("test()", execute = FALSE)
rstudioapi::sendToConsole("test(\n)", execute = FALSE)

I have tested the following combinations:

  • ✔️ radian with bracketed paste
  • ✔️ R with non-bracketed paste
  • ⚔️ radian with non-bracketed paste: In the above example, the closing parenthesis is inserted automatically

Copy link
Copy Markdown
Collaborator

@andycraig andycraig left a comment

Choose a reason for hiding this comment

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

LGTM and works as described. Thank you for the PR!

@andycraig andycraig merged commit 992560e into REditorSupport:master Mar 23, 2021
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.

Support sending to console without execution

2 participants