Remove ui_*() helper functions

[JosiahParry]

This PR closes https://github.com/extendr/rextendr/issues/177. It completely overhauls error propagation and cli messages throughout the package.

It removes R/ui.R thus removing all ui_*() functions such as ui_throw(), ui_v(), and bullet_*() counterparts. All errors are now raised using cli::cli_abort() explicitly.

One consequence of this is that errors no longer have rextendr_error class and expect_rextendr_error() has been replaced with the standard testthat::expect_error().

I think this is a fine tradeoff for increased package simplicity. I searched github and the only place that the error class was checked was in everyone's forks.

Additionally, changes to purrr changed the way that errors are raised from within (informing which iteration the error came from). I removed two instances of using map functions to ensure errors are raised properly.

[JosiahParry]

Resolved issue where quiet = TRUE was not being respected by use_extendr(). Additionally, added another test to ensure that rust_function(quiet = TRUE) is respected as well.

A note on the implementation. Previously, use_extendr() relied on a usethis option to determine if messages should be suppressed. I've modified it based on https://github.com/r-lib/cli/issues/434 which uses the cli option cli.default_handler. If quiet = TRUE this option is set to function(...) { } to print no messages per recommendation. Then, on.exit() restores the previous value (most likely NULL).

I now have no testthat failures (previously had 2).

[JosiahParry]

Alright. Most comments have been resolved. There are two major things that need to be sorted out: rextendr_error and quiet argument.

Error throwing

The big one first: this pr removes the class rextendr_error since the ui_throw() function was no longer being used. I was unaware that we can pass in arguments to rlang::abort() via ... so thanks for pointing that out.

It appears that the consensus is ensure rextendr_error class remains so that needs to be reinstituted (and thus expect_rextendr_error expectations which is no problem).

I have two questions:

1. Do we want to have a simple wrapper (less fancy than ui_throw() that adds this class? Or should it be explicitly added. I'm personally in favor of the latter. Any new PRs would need to validate that either the wrapper error function is used or the class is thrown. Idk if you can do that with lintr?
2. Do all errors in the package get the class or only functions that deal with rust and cargo directly?

Quiet

A wrapper function is desired to silence output. Again, two questions:

1. Do we prefer withr::local_options(list(cli.default_handler = function(...) {})) or withr::with_sink()
2. What is the name of the wrapper function?

[Ilia-Kosenkov]

Error throwing

Do we want to have a simple wrapper (less fancy than ui_throw() that adds this class? Or should it be explicitly added. I'm personally in favor of the latter. Any new PRs would need to validate that either the wrapper error function is used or the class is thrown. Idk if you can do that with lintr?

No strong opinion, you can do it your way.

Do all errors in the package get the class or only functions that deal with rust and cargo directly?

Yep, everything we throw ourselves should be a rextendr_error.

A wrapper function is desired to silence output. Again, two questions:

Do we prefer withr::local_options(list(cli.default_handler = function(...) {})) or withr::with_sink()

Not sure. with_sink() nukes everything, cli.default_handler specifically targets cli, so feels like a more precise tool.

What is the name of the wrapper function?

Not sure I follow, the wrapper for silencing things? No preference here

[JosiahParry]

All failing CI R CMD CHECKS contain the same warning. This is expected as the defaults no longer use usethis.

lintr checks failed because I missed a single space!

Secondly lintr wants a new line between empty curly braces. function(...) {}

    withr::local_options(
      list("cli.default_handler" = function(...) {
      })

I'll push changes for that.

── R CMD check results ──────────────────────────────── rextendr 0.2.0.9000 ────
Duration: 3m 54.8s

❯ checking for code/documentation mismatches ... WARNING
  Codoc mismatches from documentation object 'document':
  document
    Code: function(pkg = ".", quiet = FALSE, roclets = NULL)
    Docs: function(pkg = ".", quiet = getOption("usethis.quiet", FALSE),
                   roclets = NULL)
    Mismatches in argument default values:
      Name: 'quiet' Code: FALSE Docs: getOption("usethis.quiet", FALSE)

  Codoc mismatches from documentation object 'use_extendr':
  use_extendr
    Code: function(path = ".", crate_name = NULL, lib_name = NULL, quiet
                   = FALSE, edition = c("2021", "2018"))
    Docs: function(path = ".", crate_name = NULL, lib_name = NULL, quiet
                   = getOption("usethis.quiet", FALSE), edition =
                   c("2021", "2018"))
    Mismatches in argument default values:
      Name: 'quiet' Code: FALSE Docs: getOption("usethis.quiet", FALSE)

[JosiahParry]

It appears that I had the pluralization incorrect in cli from the most recent suggestion / change. It's been changed so that pluralization is calcualted in advance using "x" = "Unnamed arguments found at {cli::qty(length(invalid))} position{?s}: {invalid}."

[Ilia-Kosenkov]

I think if you sync with main to get latest CIs, it will be OK

[JosiahParry]

@Ilia-Kosenkov it has been synced with the upstream. Though there's now the question of how to handle the superceding of purrr::map_dfr()

[JosiahParry]

_dfr suffix is used twice in the code base. I think I'd opt for a follow up issue to fix this rather than solving half of it in here.

[multimeric]

Are you happy for me to merge, @JosiahParry? You probably have better awareness of the status of all the comments than I.

[JosiahParry]

@multimeric, yes! 🥂 I'll make a follow up issue regarding the purrr matter.