Skip to content

Commit

Permalink
Improve error message when using orderly interactively.
Browse files Browse the repository at this point in the history
It is quite easy and common for users to open up a file in RStudio and
start running commands from it interactively, using Ctrl-Enter, without
setting their working directory to match that file. When that happens,
orderly would throw an unhelpful error, saying only `Failed to detect
orderly path: /path/to/workspace` without any mention of working
directories, or that the path should be a report directory (as opposed
to eg. the top-level orderly workspace).

The error message is changed to look along the lines of:
```
! Working directory /path/to/workspace is not a valid orderly report.
```

This should be more helpful, by making it clear what is incorrect (the
working directory), and what was expected (an orderly report).

Additionally, if running in RStudio and the current active editor is a
file in what looks like an orderly report, it will augment the error
message with a suggestion of what `setwd` command to run:

```
! Working directory /path/to/workspace is not a valid orderly report.
ℹ Use `setwd("/path/to/workspace/src/my-report")` to set the working directory
  to the report currently open in RStudio.
```

Finally, even if the working directory is a valid report, but it does
not match the directory of the currently active editor (eg. the user has
switched from one report to another), orderly commands will succeed as
before, but will show a warning to the user:

```
Working directory /path/to/workspace/src/my-report does not match the report
currently open in RStudio.
ℹ Use `setwd("/path/to/workspace/src/other-report")` to switch working
  directories.
```

Frustratingly, while RStudio supports "click-to-run" snippets of code in
its output, it restricts the syntax of these and in particular does not
allow methods from the base package, such as `setwd`. Instead the user
has to copy-paste the suggestion into the command prompt themselves.
See rstudio/rstudio#11273 and
rstudio/rstudio#11434. The restriction is pretty
arbitrary and we could easily workaround it by re-exporting the `setwd`
function in the orderly2 package if we really wanted to.
  • Loading branch information
plietar committed Jan 18, 2024
1 parent 1f1adc4 commit bbf2a75
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 17 deletions.
1 change: 1 addition & 0 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Imports:
jsonlite,
openssl,
rlang,
rstudioapi,
withr,
yaml
Suggests:
Expand Down
52 changes: 45 additions & 7 deletions R/interactive.R
Original file line number Diff line number Diff line change
@@ -1,16 +1,54 @@
is_plausible_orderly_report <- function(path) {
path_split <- fs::path_split(path)[[1]]

length(path_split) > 2 &&
path_split[[length(path_split) - 1]] == "src" &&
file.exists(file.path(path, "../..", "orderly_config.yml"))
}

rstudio_get_current_active_editor_path <- function() {
if (rstudioapi::isAvailable()) {
rstudioapi::getSourceEditorContext()$path
} else {
NULL
}
}

## This is something that we might improve over time - it will likely
## be useful to have some sort of "register interactive" function
## which we could then just look up.
##
## I am not sure if we also want to allow working interactively from a
## draft directory too.
detect_orderly_interactive_path <- function(path = getwd()) {
path_split <- fs::path_split(path)[[1]]
is_plausible <- length(path_split) > 2 &&
path_split[[length(path_split) - 1]] == "src" &&
file.exists(file.path(path, "../..", "orderly_config.yml"))
if (!is_plausible) {
stop(sprintf("Failed to detect orderly path at '%s'", path))
detect_orderly_interactive_path <- function(path = getwd(),
editor_path = rstudio_get_current_active_editor_path()) {
is_valid <- is_plausible_orderly_report(path)
suggested_wd <- NULL
if (!is.null(editor_path)) {
dir <- fs::path_dir(editor_path)
if (paths_are_different(dir, path) && is_plausible_orderly_report(dir)) {
suggested_wd <- dir
}
}

if (!is_plausible_orderly_report(path)) {
msg <- c("Working directory {.path {path}} is not a valid orderly report.")
if (!is.null(suggested_wd)) {
cli::cli_abort(
c(msg, i = "Use {.code setwd({.str {suggested_wd}})} to set the working directory to the report currently open in RStudio."))
} else {
cli::cli_abort(msg)
}
}

if (!is.null(suggested_wd)) {
# For some reason, cli_warn has a different behaviour than cli_abort and
# doesn't make individual bullet points available in the condition object.
# The following mimicks cli_abort more closely, making testing easier.
msg <- c(
"Working directory {.path {path}} does not match the report currently open in RStudio.",
i = "Use {.code setwd({.str {suggested_wd}})} to switch working directories.")
rlang::warn(vcapply(msg, cli::format_inline), use_cli_format = TRUE)
}
as.character(fs::path_norm(file.path(path, "../..")))
}
Expand Down
9 changes: 9 additions & 0 deletions R/util.R
Original file line number Diff line number Diff line change
Expand Up @@ -632,3 +632,12 @@ saverds_atomic <- function(data, path, allow_fail = FALSE) {
finally = unlink(tmp))
}
}


paths_are_different <- function(x, y) {
tryCatch({
x_real <- fs::path_real(x)
y_real <- fs::path_real(y)
x_real != y_real
}, error = function(e) FALSE)
}
2 changes: 1 addition & 1 deletion tests/testthat/helper-orderly.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
options(outpack.schema_validate =
requireNamespace("jsonvalidate", quietly = TRUE) &&
packageVersion("jsonvalidate") >= "1.4.0",
utils::packageVersion("jsonvalidate") >= "1.4.0",
orderly_index_progress = FALSE)


Expand Down
58 changes: 49 additions & 9 deletions tests/testthat/test-interactive.R
Original file line number Diff line number Diff line change
@@ -1,18 +1,58 @@
test_that("can detect orderly directory", {
path <- test_prepare_orderly_example("explicit")
envir <- new.env()
id <- orderly_run_quietly("explicit", root = path, envir = envir)
root <- test_prepare_orderly_example("explicit")
detected_root <- detect_orderly_interactive_path(file.path(root, "src", "explicit"))
expect_equal(detected_root)
})

test_that("errors when working directory is not report", {
root <- test_prepare_orderly_example("explicit")

expect_error(
detect_orderly_interactive_path(path),
"Failed to detect orderly path at")
detect_orderly_interactive_path(root),
"Working directory .* is not a valid orderly report.")

expect_error(
detect_orderly_interactive_path(file.path(path, "src")),
"Failed to detect orderly path at")
root <- detect_orderly_interactive_path(file.path(path, "src", "explicit"))
expect_equal(path, root)
detect_orderly_interactive_path(file.path(root, "src")),
"Working directory .* is not a valid orderly report.")
})

test_that("suggests changing working directory", {
# Make matching simpler by avoiding line-wrapping.
withr::local_options(cli.width=Inf)

root <- test_prepare_orderly_example(c("explicit", "implicit"))

e <- expect_error(detect_orderly_interactive_path(
path = file.path(root, "src"),
editor_path = file.path(root, "src", "implicit", "orderly.R")),
"Working directory .* is not a valid orderly report")
expect_match(e$body[[1]], "Use `setwd(.*)` to set the working directory to the report currently open in RStudio")

w <- expect_warning(detect_orderly_interactive_path(
path = file.path(root, "src", "explicit"),
editor_path = file.path(root, "src", "implicit", "orderly.R")),
"Working directory .* does not match the report currently open in RStudio")
expect_match(w$body[[1]], "Use `setwd(.*)` to switch working directories")
})

test_that("does not unnecessarily suggest changing working directory", {
root <- test_prepare_orderly_example("explicit")

expect_no_warning(detect_orderly_interactive_path(
path = file.path(root, "src", "explicit"),
editor_path = "Untitled"
))

expect_no_warning(detect_orderly_interactive_path(
path = file.path(root, "src", "explicit"),
editor_path = file.path(root, "src", "explicit", "orderly.R")
))

expect_no_warning(detect_orderly_interactive_path(
path = file.path(root, "src", "explicit"),
editor_path = file.path(root, "orderly_config.yml")
))
})

test_that("can validate interactive parameters", {
mock_readline <- mockery::mock("TRUE", "100", "1.23", '"string"')
Expand Down

0 comments on commit bbf2a75

Please sign in to comment.