calligross / ggthemeassist

A RStudio addin for ggplot2 theme tweaking
490 stars 43 forks source link

Fallback to Shiny Widget if rstudioapi is not available #47

Closed nacnudus closed 7 years ago

nacnudus commented 8 years ago

If rstudioapi::isAvailable() is FALSE, then a new, optional argument text is used (but it has to be provided, i.e. nchar(text) > 0). Otherwise, text is updated from context$selection[[1]]$text as before.

It also closes in an alternative way if rstudioapi is not available, using stopApp(result) instead of rstudioapi::insertText(result); invisible(stopApp()). This pastes result into the console with comment characters, rather than into the script.

Another option would be, instead of the text argument, a text box in a conditional panel, for pasting code into. That would still be possible in addition to this pull request. It would be conditional on both rstudioapi being unavailable and text being empty. The text argument suits Vim purposes better since Vim can supply the text argument from selected text.

calligross commented 8 years ago

Thank you for your pull request. I will merge it soon into dev, but could you please specify under which circumstances your changes come into play? Could you point me to an IDE/setup to see how it works?

Maybe you could also make some more small changes to close the second bullet from #48:

nacnudus commented 8 years ago

Thanks for looking at it. It should come into effect in any console, even RStudio, but particularly the Windows RGui, and on Linux and Mac it would be in the terminal.

I have merged the latest changes in dev so that I could add to the error message about supplying selected text (in the RStudio case).

48 suggests a non-standard-evaluation kind of syntax ggThemeAssist(x), rather than ggThemeAssist("x"). That would be convenient for the user, but it isn't obvious how to implement it here without creating confusion. Hadley's http://adv-r.had.co.nz/Computing-on-the-language.html discusses two options: defining separate functions for standard and non-standard evaluation, or using another argument character.only = TRUE to decides how the text argument is to be evaluated. Since ggThemeAssist("x") already works, I'd be inclined to leave it alone. But this isn't my package!

I also tried setting the default value of text to be last_plot(), so that #48 could simply be ggThemeAssist(), but that made it hard to tell whether the command is being run interactively in the console or via the add-ins menu. Unfortunately, interactive() doesn't distinguish between the two. If you can suggest any other tricks then I'll gladly try them.

If you'd like me to combine all the changes into one commit, I'll go and figure out how to do that.

ottlngr commented 8 years ago

Thanks, @nacnudus . As a quite similar PR came in today I'll definitively have a closer look tomorrow.

nacnudus commented 7 years ago

I'm closing this to tidy up. Happy to reopen if there is renewed interest.