Skip to content

Commit

Permalink
Don’t fail if ‘rstudioapi’ is not installed.
Browse files Browse the repository at this point in the history
Fall back to 'tools:rstudio' inside RStudio where appropriate and
possible. Generate a warning otherwise.

Fixes #293.
  • Loading branch information
klmr committed Sep 4, 2022
1 parent 0345109 commit 461c706
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 8 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# box (development version)

* Fix: Support RStudio without installed ‘rstudioapi’ (#293)
* Fix: Make S3 method detection code more robust (#266)
* Fix: Support trailing comma in reexports (#263)
* Feature: Support lazy data loading for packages (#219)
Expand Down
42 changes: 34 additions & 8 deletions R/paths.r
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ path = function (mod) {
#' base directory, or the current working directory if not invoked on a module.
#' @rdname path
base_path = function (mod) {
normalizePath(module_path(mod), winslash = '/')
normalizePath(module_path(mod), winslash = '/', mustWork = FALSE)
}

script_path_env = new.env(parent = emptyenv())
Expand Down Expand Up @@ -183,15 +183,41 @@ testthat_path = function (...) {
#' active RStudio script file is saved.
#' @rdname path
rstudio_path = function (...) {
# `RSTUDIO` environment variable is set in terminal run inside RStudio, so
# we need to exclude that case; conversely, `.Platform$GUI` is apparently
# not yet set to "RStudio" during startup, so just checking that is
# insufficient. See comments at <https://stackoverflow.com/a/35849779/1968>.
if (Sys.getenv('RSTUDIO') != '1') return(NULL)
# .Platform$GUI is not yet set to "RStudio" during startup, so checking for
# it *might* be insufficient in corner cases.
# However, we cannot merely check whether the `RSTUDIO` environment
# variable is set, because it is also set inside the RStudio terminal; and
# when ‘box’ is invoked from a script that is run in the terminal, we do
# *not* want to use RStudio’s active document, since that isn’t the script
# from which we are called.
# See also comments at <https://stackoverflow.com/a/35849779/1968>.
if (! identical(.Platform$GUI, 'RStudio')) return(NULL)

document_path = rstudioapi::getActiveDocumentContext()$path
if (! identical(document_path, '')) dirname(document_path)
document_path = if (requireNamespace('rstudioapi', quietly = TRUE)) {
rstudioapi::getActiveDocumentContext()$path
} else {
# ‘rstudioapi’ might not be installed. Attempt to use the internal API
# as a fallback.
if ('tools:rstudio' %in% search()) {
as.environment("tools:rstudio")$.rs.api.getActiveDocumentContext()$path

This comment has been minimized.

Copy link
@klmr

klmr Sep 5, 2022

Author Owner

Wrap in tryCatch(…): currently the code accounts for changes to the environment name, but not to the internal API inside that environment.

} else {
warning(fmt(
'It looks like the code is run from inside RStudio but',

This comment has been minimized.

Copy link
@klmr

klmr Sep 5, 2022

Author Owner

Add missing spaces between arguments.

'{"box";\'} is unable to identify the calling document. This',
'should not happen. Please consider filing a bug report at',
'<https://github.com/klmr/box/issues/new/choose>.'
))
return(NULL)
}
}

if (identical(document_path, '')) {
# The active document wasn’t saved yet, or the code is invoked from the
# R REPL/console.
getwd()
} else {
dirname(document_path)
}
}

#' @return \code{wd_path} returns the current working directory.
Expand Down
70 changes: 70 additions & 0 deletions tests/testthat/test-rstudio.r
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
context('RStudio')

with_mock_rstudio = function (expr) {
Sys.setenv(RSTUDIO = "1")

old_gui = .Platform$GUI
unlockBinding('.Platform', .BaseNamespaceEnv)
.BaseNamespaceEnv$.Platform$GUI = 'RStudio'
Sys.unsetenv('TESTTHAT')

on.exit({
Sys.unsetenv("RSTUDIO")
.BaseNamespaceEnv$.Platform$GUI = old_gui
lockBinding('.Platform', .BaseNamespaceEnv)
Sys.setenv(TESTTHAT = 'true')
})

expr
}

with_mock_rstudio_tools_path = function (path, expr) {
rstudio_tools_env = new.env()
local(envir = rstudio_tools_env, {
.rs.api.getActiveDocumentContext = function () { # nolint
list(
path = path,
contents = '',
selection = list(list(range = rep(1L, 4L), text = ''))
)
}
.rs.api.versionInfo = function () list()
})
on.exit(detach('tools:rstudio'))
attach(rstudio_tools_env, name = 'tools:rstudio')

expr
}

test_that('Path of active file in RStudio is found without ‘rstudioapi’', {
skip_on_ci()
skip_on_cran()
# Assume that we cannot edit package library path on other systems.
skip_outside_source_repos()

pkg_path = dirname(dirname(attr(suppressWarnings(packageDescription('rstudioapi')), 'file')))
tmp_path = paste0(pkg_path, '-x')

unloadNamespace('rstudioapi')
expect_true(file.rename(pkg_path, tmp_path))
on.exit(file.rename(tmp_path, pkg_path))

file_path = with_mock_rstudio({
with_mock_rstudio_tools_path('/rstudio/test.r', box::file())
})

expect_paths_equal(file_path, '/rstudio')
expect_false(isNamespaceLoaded('rstudioapi'))
})

test_that('Path of active file in RStudio is found with ‘rstudioapi’', {
skip_on_cran()
skip_if(is.na(packageDescription('rstudioapi')))

This comment has been minimized.

Copy link
@klmr

klmr Sep 5, 2022

Author Owner

Should use skip_if_not_installed.


file_path = with_mock_rstudio({
with_mock_rstudio_tools_path('/rstudio/test.r', box::file())
})

expect_paths_equal(file_path, '/rstudio')
expect_true(isNamespaceLoaded('rstudioapi'))
})

0 comments on commit 461c706

Please sign in to comment.