-
Notifications
You must be signed in to change notification settings - Fork 9
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
verbose
= !quiet
#284
verbose
= !quiet
#284
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't check why tests fail but one comment
R/render_docs.R
Outdated
# Quarto sometimes raises errors encouraging users to set `quiet=FALSE` to get more information. | ||
# This is a convenience check to match Quarto's `quiet` and `altdoc`'s `verbose` arguments. | ||
dots <- list(...) | ||
if ("quiet" %in% names(dots) && is.logical(dots[["quiet"]]) && length(dots[["quiet"]]) == 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the length condition should be before the is.logical()
to ensure we won't have an issue when is.logical()
returns a vector of length > 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. But I think the proper fix is to have the length
check in isTRUE()
, because length()
might not work at all or weirdly on objects of certain class.
I'll fix the test once the other PR is merged. |
When Quarto breaks, it encourages users to call the same command with
quiet=FALSE
. However, the analogous command inaltdoc::render_docs()
is calledverbose
. I keep getting caught by this, so thought I would implement a simple check to assignverbose <- !quiet
when the user usesquiet
in...
This is hard to test, and I don't think we should advertise this feature. But it's a trivial change, so easy to review.