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

Metaprogramming: extracting formatted objects #750

Open
MichaelChirico opened this issue Feb 20, 2025 · 7 comments
Open

Metaprogramming: extracting formatted objects #750

MichaelChirico opened this issue Feb 20, 2025 · 7 comments

Comments

@MichaelChirico
Copy link
Contributor

MichaelChirico commented Feb 20, 2025

To support interpreting {cli} calls in lintr::object_usage_linter() & thereby prevent false positives related to "unused variables", we'd like to examine relevant calls & extracting out the used variables.

r-lib/lintr#2252

We do something similar for {glue} already, but the language for {cli} is similar-but-much-richer, e.g. {glue} does not support {.arg x}-style markup.

For {glue}, we use the .transformer argument to pull out all the names in {} execution regions inside glue() calls:

https://github.com/r-lib/lintr/blob/07c55846c1189740b7ac7a83b4b53203e23940ca/R/shared_constants.R#L264-L272

Is there any equivalent for {cli}? I saw #152 but I don't think cli_format_method() is quite suitable.

To be a bit clearer, here's some sample input/output:

cli_abort("{.arg x} has {length(x)} elements, but it should be {.emph {length(y)}}")
# --> c("x", "y")

We might be able to do something where we regex our way through and delete the {.markup ...} wrappers, but I think that's less than ideal.

@gaborcsardi
Copy link
Member

I think the best would be to add an option or function to cli to do this.

@MichaelChirico
Copy link
Contributor Author

OK, glad to know I'm not missing something.

Any initial thoughts on an API? something similar to the {glue} approach? I'm not all that familiar with {cli} internals.

cc @olivroy who has helped out with some related issues here.

@gaborcsardi
Copy link
Member

I would add a cli_extract_vars() (internal?) function.

@MichaelChirico
Copy link
Contributor Author

Looked at the implementation a little bit. AIUI, we'd want to use all.vars() (or all.names()) on expr here:

cli/R/inline.R

Line 303 in b388132

expr <- parse(text = code, keep.source = FALSE) %??%

I'm still not sure the right way to surface that.

I also see that all of the functions like cli_abort() that support inline formatting are doing so through a call to cli::format_inline().

WDYT about a new argument like .record_names=FALSE to that function which would then alter how glue() is invoked?

@gaborcsardi
Copy link
Member

I also see that all of the functions like cli_abort() that support inline formatting are doing so through a call to cli::format_inline().

That would be very surprising to me. I would think almost none of them call format_inline().

@MichaelChirico
Copy link
Contributor Author

Oh, my mistake :)

I only checked one

cli/R/rlang.R

Lines 39 to 44 in b388132

cli_abort <- function(message,
...,
call = .envir,
.envir = parent.frame(),
.frame = .envir) {
message[] <- vcapply(message, format_inline, .envir = .envir)

@MichaelChirico
Copy link
Contributor Author

In that case the workhorse is actually glue_cmd(), which is currently private.

Maybe that could be exported with the suggested .record_names option instead?

Another idea is an option to strip {cli}-specific markup instead, returning a string that could be passed directly to {glue} with the {cli} logic removed, I think that means returning code in the above branch.

"{.arg x} has {length(x)} elements, but it should be {.emph {length(y)}} in {.pkg {.emph {pkg}}}"
# --> "x has {length(x)} elements, but it should be {length(y)} in {pkg}"

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

No branches or pull requests

2 participants