search

yihui / knitr

A general-purpose tool for dynamic report generation in R
https://yihui.org/knitr/
2.37k stars 874 forks source link

No specific processing when inside quarto for special collapse ASIS TOKEN #2248

Closed cderv closed 1 year ago

cderv commented 1 year ago

to fix quarto-dev/quarto-cli#5052

I believe the issue is that we previously did not run as.character() when inside htmlwidget processing https://github.com/yihui/knitr/blob/a007b376174ea080ae8ab4668509535dea30a5f5/R/output.R#L481-L487 We were passing x

now we run https://github.com/yihui/knitr/blob/99cd65a868b1d0e41c69b208aac9178ad1a1c309/R/output.R#L494 with https://github.com/yihui/knitr/blob/99cd65a868b1d0e41c69b208aac9178ad1a1c309/R/output.R#L467-L475

And Quarto does a brute force replacement in namespace https://github.com/quarto-dev/quarto-cli/blob/4977f4501632de13a92cca6cfebac26f4baa2326/src/resources/rmd/patch.R#L111-L118

So the x received in not the same.

Quick solution as 1.3 will go out really soon, is to ignore quarto when doing the processing. Especially because there is no issue. See https://github.com/yihui/knitr/pull/2212#pullrequestreview-1292924523

collapse = TRUE with htmlwidget wokrs with Quarto because Quarto is already wrapping (for other purpose) any knit_asis output, and especially HTMLwidgets by using hooks and replacement function. Example of what knitr receive before collapse.

@yihui Do you think this is enough, or we should rethink the fix ?

BTW I found this issue by chance following a report on community. We should really consider running Quarto checks with the dev version of knitr before doing a release. I'll add some run of the Quarto checks with dev knitr too.

yihui commented 1 year ago

It seems we are having trouble with an rgl demo in the knitr-examples repo, but we can figure it out later. I'll merge this PR first.