Throw classed error if allow_empty = FALSE + add more context to error message by adding error_arg to eval_select()

[olivroy]

Closes #347

basically implement what was raised in #282 (comment)

Addresses part of #327

I have to say this fix is a bit naive and narrow and lacks a more general view.

pkgload::load_all() 
#> ℹ Loading tidyselect
# give a better error message if you don't want empty selection
 # if supplying `error_arg`
 select_not_empty <- function(x, cols) {
   eval_select(expr = enquo(cols), data = x, allow_empty = FALSE, error_arg = "cols")
 }
 try(select_not_empty(mtcars, cols = starts_with("vs2")))
#> Error in select_not_empty(mtcars, cols = starts_with("vs2")) : 
#>   `cols` must select at least one column.

^{Created on 2024-04-25 with reprex v2.1.0}

Would it make sense to recognize it in eval_relocate()?

A version I had locally had error_arg = c(before_arg, after_arg), but couldn't tell if it was useful.

This implementation doesn't change the default, which mean it is opt-in.

Reason behind.

I was working on gt and discovered allow_empty and I thought I'd use it to throw messages, (without having to rethrow it again)

But I found that the error message Can't select empty items was not informative at all, so it is required to rethrow it or make tweaks in tidyselect.

So I thought it would be great if tidyselect had a mechanism to do this, and discovered it was proposed last time tidyselect was updated, but was never implemented.

[lionel-]

Thanks for taking a look! Just have some comments, please let me know if you need me to finish this up.

[lionel-]

This should mention errors in general, as it might be used more generally in the future.

Should also reference it's for expr, and that it can be set to "..." if called with expr = c(...).

[lionel-]

Since you use .or, mention this can be a character vector of argument names?

[olivroy]

ideally, it should never resolve to being multiple arguments. Since before and after being both supplied should result in another error message.. (can't supply after and before

[lionel-]

oh yeah, makes sense!

[lionel-]

Need relocation tests as well?

[olivroy]

Added the argument to eval_relocate() as well. I realized that before_arg and after_arg are not involved in all this. before and after will error elsewhere if they end up being empty.

Edit: See also https://github.com/tidyverse/tidyr/pull/1549/files where I attempted to use this! I think a few packages will benefit from this once released to CRAN.

[lionel-]

Thank you!