MilesMcBain / datapasta

On top of spaghetti, all covered in cheese....
https://milesmcbain.github.io/datapasta/
Other
891 stars 58 forks source link

Use Rstudio editor selection as fallback when clipboard not available #104

Closed gadenbuie closed 4 years ago

gadenbuie commented 4 years ago

Following up on the conversation we had last night (your morning I guess) on Twitter. I was going to open a long detailed issue but then figured it'd just be easier to write out the code. Of course, I was probably wrong; I ran into a couple twists that I hadn't expected but I think I have them sorted out.

In short, this PR refactors the clipboard reading step into a single function that will attempt to fall back to the current selection in the RStudio editor if the clipboard isn't available. This makes datapasta seamless on RStudio Server.

datapasta-preview

The overall logic is to test if clipr::clip_available() and if it is then to read from the clipboard. If not, then try reading using the rstudioapi. If that's not available (not in RStudio) then throw the typical clipboard not available error. If we're in RStudio but nothing is selected, then inform the user that they can paste into the editor, select the pasted text, and re-run the addin.

The RStudio API gives a different format than clipr::read_clip() so I wrote the selection to a temp file that I read in with scan(), using the same line as in clipr:::osx_read_clip() and clipr:::X11_read_clip(). The temp file is deleted immediately so I don't think this will cause problems with CRAN.

To test the PR, at least interactively, I discovered that you can set

Sys.setenv("CLIPR_ALLOW" = FALSE)

to temporarily disable clipr and force datapasta to fall back on RStudio. I didn't add any tests, but I did make sure that this PR doesn't break any (more) tests (than are currently failing).

cc @jonthegeek the above gif is from an RStudio Server šŸ˜ƒ

jonthegeek commented 4 years ago

Ooooh, that's awesome! That will definitely cover my use case!

It looks like there's something causing an error in the documentation, though...

gadenbuie commented 4 years ago

Awesome!

The checks are failing because I didn't re-render the documentation (didn't want to clutter up the PR)

gadenbuie commented 4 years ago

Spurred by my desire to help @jonthegeek avoid carpal tunnel with crazy keyboard shortcuts, I added another final step that opens an editor with utils::file.edit() for a temp file where the user can just paste their text. The overall chain is then clipboard > selection > text prompt.

datapasta-preview-2

gadenbuie commented 4 years ago

I added an option to disable the instructions prompt by setting

options(datapasta.quiet_manual_paste = TRUE)

and I also set it to muffle the instructions after the first prompt per session. (Note I haven't documented this anywhere, not sure where best to put that information.)

jonthegeek commented 4 years ago

and I also set it to muffle the instructions after the first prompt per session. (Note I haven't documented this anywhere, not sure where best to put that information.)

Hmm. I'm not sure about addin documentation in general (definitely something I should learn if it's a thing), but I can imagine a vignette along the lines of "Using datapasta in a browser". Maybe THAT will be left to do on Monday? šŸ˜

gadenbuie commented 4 years ago

It occurred to me that utils::file.edit() doesn't need to be in RStudio, so we could also use the "paste into editor" workflow on terminals where the clipboard isn't available, like a remote server via SSH.

The new "paste" flow looks like this: clipboard > (RStudio? try selection) > text prompt. Here's a short asciicast demo.

asciicast

MilesMcBain commented 4 years ago

Thanks for this @gadenbuie! I just noticed your fork is quite behind, which explains the test failures. There are none on master currently.

Any advice on how to proceed here? I could change the target branch to a copy of master and pull it there for testing... or you could resync your fork and make a new PR?

gadenbuie commented 4 years ago

ugh. Sorry about that! I use dev in my projects for pre-release code; I guess I just ran with that.

It looks like conflicts are limited primarily to df_paste.R ... it's hard to see on GitHub but I'm pretty sure I can resolve manually. If not I'll rebase my branch on master and open a new PR.

MilesMcBain commented 4 years ago

Ohhhh I see. Easy misake to make. I've got a deadwood cleanup to do.

gadenbuie commented 4 years ago

Okay, so that wasn't too bad! I'm shocked that the PR is still readable. I merged in master and fixed the few minor conflicts so this should be up to date now.

Most of what I wrote is hard to test, but I did extract the function that reads the text gathered by the fallback methods so that we can test that clipr::read_clip() and this method give us the same answer.

MilesMcBain commented 4 years ago

This is really nice work, thanks for the crisp PR @gadenbuie!