Error messages using cli may wrap incorrectly for S7 property validators

[dmuenz]

In an S7 property validator, the error message is supposed to be "a single string" according to the docs, but I noticed that ggplot2 sometimes violates this. When you use {cli} to format the error message, it may cause a long message to wrap across multiple lines. Each line is then a separate string in a multi-string vector.

The consequence of this is that each line of the error message is prefaced with - @<property_name>, making it look perhaps like several error messages instead of just one long one. Here's an example (note that the line wrapping depends on the console width):

ggplot2::element_text(face = "foo")
#> Error: <ggplot2::element_text> object properties are invalid:
#> - @face must be one of "plain", "bold", "italic", "oblique", or
#> - @face "bold.italic".

I think this would be easier to read as:

#> Error: <ggplot2::element_text> object properties are invalid:
#> - @face must be one of "plain", "bold", "italic", "oblique", or
#>   "bold.italic".

If you agree that this is a (admittedly very small) problem, I think it could be fixed by revising your internal as_cli function to use paste(collapse), so the output would be a single string. Something like this:

as_cli <- function(..., env = parent.frame(), collapse = NULL) {
  cli::cli_text(..., .envir = env) |>
    cli::cli_fmt() |>
    paste(collapse = collapse)
}

long_string <- "I am a very {.emph long} string and I just keep going and going but now I'm done"

as_cli(long_string)
#> [1] "I am a very \033[3mlong\033[23m string and I just keep going and going"
#> [2] "but now I'm done"                                                      

as_cli(long_string, collapse = "\n")
#> [1] "I am a very \033[3mlong\033[23m string and I just keep going and going\nbut now I'm done"

Note that this preserves as_cli's current behavior if you leave collapse = NULL . When using this for S7 property validators, you could perhaps do as_cli(long_string, collapse = "\n ") to indent the wrapped line a bit.

Anyway, this is me being super picky about a very minor issue. Thanks for your consideration.

[teunbrand]

Thanks for the report!

Anyway, this is me being super picky about a very minor issue.

As long as there is also a viable path to improvement envisioned, this is not a problem at all! I think the cli_fmt(collapse) argument can also be employed here. Are you interested in preparing a PR for this?

[dmuenz]

In contrast to paste(collapse), I think the cli_fmt(collapse) argument is just a TRUE/FALSE flag, meaning collapse or not. That could convert to a single string as desired, but there’s no flexibility to indent the new line. So I think using paste(collapse) is a better option.

I’m interested in preparing a PR (thank you for asking!), but this would be my first one, so I’d need to first learn the process.

[teunbrand]

I don't think the indentation of new lines matters too much for ggplot2 and would depend on the console width for printing anyway.

If you're used to working in R (as opposted 'instistent on doing git from the command line'), there is a nice vignette in usethis that describes the process.