Skip to content

Commit

Permalink
Supply both before_arg and after_arg to error_arg.
Browse files Browse the repository at this point in the history
Change class of error message

Use new comment.
  • Loading branch information
olivroy committed Oct 22, 2024
1 parent b369821 commit e063ac7
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 15 deletions.
1 change: 1 addition & 0 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,4 @@ Config/Needs/website: tidyverse/tidytemplate
Config/testthat/edition: 3
Encoding: UTF-8
RoxygenNote: 7.3.2
Roxygen: list(markdown = TRUE)
3 changes: 1 addition & 2 deletions R/eval-relocate.R
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ eval_relocate <- function(expr,
data <- tidyselect_data_proxy(data)

expr <- as_quosure(expr, env = env)

sel <- eval_select_impl(
x = data,
names = names(data),
Expand All @@ -89,7 +88,7 @@ eval_relocate <- function(expr,
allow_empty = allow_empty,
allow_predicates = allow_predicates,
type = "relocate",
error_arg = NULL, # TODO need to know which to use. can `before_arg` or `after_arg` be passed here?
error_arg = c(before_arg, after_arg),
error_call = error_call
)

Expand Down
5 changes: 3 additions & 2 deletions R/eval-select.R
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@
#' # Note that the trick above works because `expr({{ arg }})` is the
#' # same as `enquo(arg)`.
#'
#' # give a better error message if you don't want empty selection
#' # if supplying `error_arg`
#' # Supply `error_arg` to improve the error message in case of
#' # unexpected empty selection:
#' select_not_empty <- function(x, cols) {
#' eval_select(expr = enquo(cols), data = x, allow_empty = FALSE, error_arg = "cols")
#' }
Expand Down Expand Up @@ -182,6 +182,7 @@ eval_select_impl <- function(x,
if (is_null(names)) {
cli::cli_abort("Can't select within an unnamed vector.", call = error_call)
}

# Put vars in scope and peek validated vars
local_vars(names)
vars <- peek_vars()
Expand Down
6 changes: 3 additions & 3 deletions R/eval-walk.R
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,13 @@ check_empty <- function(x, allow_empty = TRUE, error_arg = NULL, call = caller_e
cli::cli_abort(
"Must select at least one item.",
call = call,
class = "tidyselect:::error_disallowed_empty"
class = "tidyselect_error_empty_selection"
)
} else {
cli::cli_abort(
"{.or {.arg {error_arg}}} must select at least one column.",
"{.arg {error_arg}} must select at least one column.",
call = call,
class = "tidyselect:::error_disallowed_empty"
class = "tidyselect_error_empty_selection"
)
}
}
Expand Down
4 changes: 2 additions & 2 deletions man/eval_select.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 21 additions & 6 deletions tests/testthat/_snaps/eval-relocate.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,21 +84,36 @@
Code
(expect_error(relocate_loc(x, allow_empty = FALSE)))
Output
<error/tidyselect:::error_disallowed_empty>
<error/tidyselect_error_empty_selection>
Error in `relocate_loc()`:
! Must select at least one item.
! `before` and `after` must select at least one column.
Code
(expect_error(relocate_loc(mtcars, integer(), allow_empty = FALSE)))
Output
<error/tidyselect:::error_disallowed_empty>
<error/tidyselect_error_empty_selection>
Error in `relocate_loc()`:
! Must select at least one item.
! `before` and `after` must select at least one column.
Code
(expect_error(relocate_loc(mtcars, starts_with("z"), allow_empty = FALSE)))
Output
<error/tidyselect:::error_disallowed_empty>
<error/tidyselect_error_empty_selection>
Error in `relocate_loc()`:
! Must select at least one item.
! `before` and `after` must select at least one column.

---

Code
(expect_error(relocate_loc(mtcars, before = integer(), allow_empty = FALSE)))
Output
<error/tidyselect_error_empty_selection>
Error in `relocate_loc()`:
! `before` and `after` must select at least one column.
Code
(expect_error(relocate_loc(mtcars, starts_with("z"), allow_empty = FALSE)))
Output
<error/tidyselect_error_empty_selection>
Error in `relocate_loc()`:
! `before` and `after` must select at least one column.

# `before` and `after` forbid renaming

Expand Down
10 changes: 10 additions & 0 deletions tests/testthat/test-eval-relocate.R
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,16 @@ test_that("can forbid empty selections", {
})
})

test_that("can forbid empty selections", {
x <- c(a = 1, b = 2, c = 3)

expect_snapshot({
(expect_error(relocate_loc(mtcars, before = integer(), allow_empty = FALSE)))
(expect_error(relocate_loc(mtcars, starts_with("z"), allow_empty = FALSE)))
})
})


test_that("`before` and `after` forbid renaming", {
x <- c(a = 1, b = 2, c = 3)

Expand Down

0 comments on commit e063ac7

Please sign in to comment.