Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Augment Abort & co.; add rlang condition tutorial, helpers #102

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

brookslogan
Copy link
Contributor

This is a draft. The additional Abort&co. features still need to have testthat tests, and to be used throughout the package.

Motivation: we "use" rlang conditions, but

  • don't use any of its formatting, error class name, and error object field-setting features.
  • this breaks the options(error=recover) debugging workflow in at least some R versions
  • many users may be unaware of some of the features of rlang conditions from just following the prompts to rlang::last_error() and rlang::last_trace()

We want to help users get accustomed to rlang conditions, and take advantage of more of its features; building on Abort and Warn seems like a good way.

Changes so far:

  • Add Inform, similar to Abort and Warn
  • Add class_suffix, display_subfields, more_subfields, and call to
    Abort, Warn, Inform
  • On first raised condition, provide a tutorial on rlang conditions to help
    users with compatibility issues and potentially-unknown features
  • Improve condition traces via call default
  • Improve condition message line wrapping, compatibility with messages with
    bullet points, etc.

To complement the tutorial, some convenience functions for working with global calling handlers, as well as some handler functions, are provided.

- Add `Inform`, similar to `Abort` and `Warn`
- Add `class_suffix`, `display_subfields`, `more_subfields`, and `call` to
  `Abort`, `Warn`, `Inform`
- On first raised condition, provide a tutorial on `rlang` conditions to help
  users with compatibility issues and potentially-unknown features
- Improve condition traces via `call` default
- Improve condition message line wrapping, compatibility with messages with
  bullet points, etc.
# inside this template tag file, leading to long `r ...` chunks; don't
# wrap/fill/format these, or else current roxygen2 at time of writing will not
# properly process them. Keep the blank lines around them to help prevent
# accidental auto-formatting.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roxygen templates provide a different way to execute dynamic code with <%= ...... %>. Might provide a way around the line wrap fighting. (Here and in other template file, and maybe also in other non-template places where dynamic R code is used that could be turned into templates.)

#'
#' @importFrom rlang caller_env is_reference global_env base_env empty_env
#' is_function
#' @importFrom tibble lst
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might need @noRd here and in other internal helper functions. Not the handler ones though, as they will be referenced by the tutorial.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vs. @keywords internal?

@brookslogan
Copy link
Contributor Author

brookslogan commented Jun 22, 2022

  • Printing of more debug fields might need to be done via encodeString. this doesn't do the conditional backtick quoting I initially thought it was supposed to do (too hastily read the first part of the docs). Perhaps there is some sort of encoding correctness to think about, but it'd probably be across the entire package as best practices rather than something specific here.

@brookslogan
Copy link
Contributor Author

brookslogan commented Jun 23, 2022

  • For migration from existing Abort etc., should allow class parameter as an alternative to class_prefix parameter.
  • Resolve merge conflicts.
  • To prevent strange tutorial placement especially with some warning spam, consider only showing tutorial by default for Abort.
  • Add tests.
  • Use throughout package + add issue to use in code developed in parallel with these changes.
  • Allow debug info display function to be customizable rather than hard-coded as str.
  • Consider pointing to and using rlang::try_fetch rather than withCallingHandlers and/or tryCatch.
  • Consider moving from templates to @eval of functions returning roxygen tags. See here.
  • Use cat_varnames (under dev in lcb/grouped_epi_archive) for printing subfield names
  • Check usage of nchar: check whether/which calls should use type="width"
  • Check naming of check_* functions / change them to make them idiomatic
  • Go ahead and move to cli_* or use_cli_format since we transitively import it anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide more informative/helpful condition messages in Abort, Warn(, Inform?)
3 participants