Skip to content

Commit

Permalink
Fix CI issue caused by new cli package.
Browse files Browse the repository at this point in the history
Recent versions of the cli package have added a new `vec-sep2` style
option for `cli_vec`, in an effort to improve their handling of Oxford
commas. This means `vec-last` is now ignored for any list of length two,
and the value of `vec-sep2` is used instead.

We only used that feature in one place, with "or" as the value for
`vec-last`. The new cli package broke a test by making that code print
the default `vec-sep2` value, i.e. "and", when there are only two
elements.

After looking at the context of that message however, I think "and"
actually makes more sense than "or". It is printing a list of paths that
haven't been found. We would want all of the paths in the list to be
present, not just one of them. The simplest fix is therefore to remove
the call to `cli_vec` and let cli use the default settings.

Finally, remove a call to `squote` on the missing paths and use a
`.path` tag in the message. The end result is the same.
  • Loading branch information
plietar committed Jan 18, 2024
1 parent 1f1adc4 commit c938c59
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 3 deletions.
3 changes: 1 addition & 2 deletions R/outpack_root.R
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,8 @@ validate_packet_has_file <- function(root, id, path, call = NULL) {
}
}

vmsg <- cli::cli_vec(squote(msg), list("vec-last" = " or "))
err <- paste("Packet '{id}' does not contain the requested",
"{cli::qty(vmsg)} path{?s} {vmsg}")
"{cli::qty(msg)} path{?s} {.path {msg}}")
cli::cli_abort(c(err, set_names(hint, "i")), call = call)
}

Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-outpack-helpers.R
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ test_that("good error message if file not found in packet", {

err <- expect_error(
validate_packet_has_file(root, id, c("a.txt", "a.TXT", "d.txt")),
"Packet '.+' does not contain the requested paths\\s*'a.TXT' or 'd.txt'")
"Packet '.+' does not contain the requested paths\\s*'a.TXT' and 'd.txt'")
expect_equal(
err$body,
c(i = "For 'a.TXT' did you mean 'a.txt'",
Expand Down

0 comments on commit c938c59

Please sign in to comment.