Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make is_interactive() react to "rlang_interactive" option #734

Merged
merged 3 commits into from
Feb 22, 2023

Conversation

jennybc
Copy link
Member

@jennybc jennybc commented Feb 22, 2023

This will allow me to make gargle's pseudo-OOB flow work in Google Colab, which would be really nice.

Relates to r-lib/gargle#237, where I currently have utils::assignInNamespace("is_interactive", rlang::is_interactive, ns = "httr") in .onLoad(), but I can remove that if this PR gets merged and released. I don't think I could release gargle to CRAN with assignInNamespace().

Relates to r-lib/gargle#140

Comment on lines +217 to +219
is_interactive <- function() {
getOption("rlang_interactive") %||% interactive()
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I can't use rlang::is_interactive() because httr does not depend on rlang at this point. And it doesn't seem worth bringing a whole standalone file in for this.

Comment on lines +114 to +117
old <- options()
on.exit(options(old))
options(rlang_interactive = FALSE)

Copy link
Member Author

Choose a reason for hiding this comment

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

This test would previously fail when run with test_active_file(), so I fixed that while I was here.

Comment on lines +124 to +126
old <- options()
on.exit(options(old))
options(rlang_interactive = TRUE)
Copy link
Member Author

Choose a reason for hiding this comment

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

Took the chance to get rid of the with_mock() call, even though httr doesn't use testthat 3e. httr also doesn't use withr.

@jennybc
Copy link
Member Author

jennybc commented Feb 22, 2023

Just to record this analysis somewhere: I have not replaced every call to base::interactive() with is_interactive(). Why not? Where else is base::interactive() called?

  • BROWSE(): For my motivating case, which is Google Colab, utils::browseURL() does not work and we do indeed want to go down the non-interactive() branch.
  • should_cache() (internal helper): gargle never hits this code, because caching is done in-house. But FWIW utils::menu() really does require a fully interactive session.
  • Default args in init_oauth1.0(), init_oauth2.0(), oauth_listener(): these args are deprecated, so it doesn't matter.

@jennybc jennybc merged commit 849a3e8 into main Feb 22, 2023
@jennybc jennybc deleted the use-rlang-is-interactive branch February 22, 2023 17:02
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.

2 participants