Add knit_cnd_format() generic

[lionel-]

rlang is now in charge of fully printing error messages, including the error prefix. This makes it possible to support more features, such as custom display of condition calls and chained errors.

mtcars |> dplyr::summarise(cyl = 1 + "")
#> Error in `dplyr::summarise()`:
#>   Problem while computing `cyl = 1 + ""`.
#> Caused by error in `+`:
#>   non-numeric argument to binary operator

Ideally, the condition messages in knitted documents should match the messages in the REPL. This is currently achieved for errors via an as.character.rlang_error() method that is called by sew.error(). However, methods for other conditions do not call as.character(). Also that customisation point is not very explicit and is probably to be considered off-label usage from the point of view of knitr.

To solve these issues, this PR proposes to add a new generic for the full formatting of condition messages, including the prefix. Example usage can be found in https://github.com/r-lib/rlang/pull/1323/files.

cc @hadley

[CLAassistant]

All committers have signed the CLA.

[cderv]

Just a small feedback as I observe a change in the test.

Otherwise, this looks good. I let @yihui add his review which will be more relevant for knitr internals. Thanks Lionel !

[cderv]

I think the paste() here was required so that this works

A message in message(..., appendLF = FALSE) will be merged with the next adjacent message:

f3 = function() {
 message('Hello ', appendLF = FALSE)
 message('World!')
}
f3()

The change here currently make the test fails on this. The knitr-examples checks is showing the difference between previous and new behavior for example (https://github.com/yihui/knitr-examples/blob/master/117-messages.Rmd)
https://github.com/yihui/knitr/runs/4396175038?check_suite_focus=true#step:11:1669

Was it necessary to remove it in knit_cnd_format.message ?

[lionel-]

Thanks for the pointer Christophe!

Since it's not "in-API" to return a character vector from conditionMessage(), I overlooked that part. Now fixed, with unit test and comments. However this means that knit_cnd_format() methods for messages need to know about that message merging behaviour and the message field potentially being a character vector. That's a bit problematic.

By the way, merge_class() manipulates message fields instead of the result of conditionMessage(). This could cause issues in the future because conditions might have empty "" or dummy strings as message fields, with the actual message generated with a conditionMessage() method. It's not a big deal though because lazy generation of error messages mostly occurs with errors not messages. However it's worth knowing that logic isn't 100% correct.

I guess ideally knit_cnd_format() would be called inside the merge_class() routine so that it generates messages properly. A new object holding the merged messages would need to be created instead of modifying the existing condition, which is not hygienic because it may break class expectations. And then we don't need the collapsing in knit_cnd_format.message() or any methods for subclasses of messages.

That said, we don't need to fix this now (or maybe ever) since message subclasses are not that important.

[lionel-]

By the way, is there a way to run the knitr-examples check locally?

[cderv]

To run them locally, you need to clone the repository and then knit the files.
Usually, we automate the testing using CI and only run locally when something seems off.

Otherwise, to reproduce the checks without trying to recreate the full environnemnt (not that easy, that is why I rely on CI as I struggled bc on Windows mainly), the script to run in knitr-examples/ is ./knitall after having installed dev version.
The full process is make integration in knitr folder, which should work if you have knitr-example at the right place inside the knitr folder. (what happens in the GHA workflow).

To run one file, the script is ./k

Anyway, relying on the CI to run those tests is fine also.