rstudio / bookdown

Authoring Books and Technical Documents with R Markdown
https://pkgs.rstudio.com/bookdown/
GNU General Public License v3.0
3.78k stars 1.27k forks source link

Raw HTML in a <div> with class "solution" causes a Lua filter error #1172

Open debruine opened 3 years ago

debruine commented 3 years ago

A bookdown chapter with the lone content below triggers the error below that. It requires the div be named "solution" and there to be a button inside (this is created by a function from the webex package that is made for use with bookdown, so is actually causing a lot of problems).

<div class='solution'>
<button>Solution</button>
</div>

Error running filter /Library/Frameworks/R.framework/Versions/4.1/Resources/library/bookdown/rmarkdown/lua/custom-environment.lua:...es/library/bookdown/rmarkdown/lua/custom-environment.lua:183: bad argument #1 to 'insert' (table expected, got nil)stack traceback: [C]: in function 'pandoc.List.insert' ...es/library/bookdown/rmarkdown/lua/custom-environment.lua:183: in function 'Div'Error: pandoc document conversion failed with error 83

See https://twitter.com/chrisderv/status/1402323292430770178 by @cderv

cderv commented 3 years ago

Thanks for the report.

Context about special Theorem & Proof env

For context, bookdown handles Solution as a Proof environment since #423 - this means that this is processed by bookdown as a special environment to create a Proof like Solution environment in HTML and LaTeX (like the amsmath one)

::: solution
This is the solution to the problem mentioned above
:::

image

Using <div class='solution'> leads to the same because of Pandoc's native_divs extension activated by default.

Why this issue happens ?

This is issue mentioned above is thrown by our Lua filter because of this special handling of Div of class Solution. We have this issue because the type RawBlock in Lua does not behave like other Pandoc types: it does not have content

pandoc -t native
<div class='solution'>
<button>Solution</button>
</div>
^Z
[Div ("",["solution"],[])
 [RawBlock (Format "html") "<button>"
 ,Plain [Str "Solution"]
 ,RawBlock (Format "html") "</button>"]]

https://github.com/rstudio/bookdown/blob/c8883c9bbd70400144b396b7cfc66e5e4f2ae8a5/inst/rmarkdown/lua/custom-environment.lua#L195-L196

div.content[1] is a RawBlock and div.content[1].content does not exist. See Lua Filter doc - RawBlock only have format, text and t.

What could be done ?

There would be several way to fix this:

It was not really expected to have raw HTML inside this type of environment. I don't even know if it makes sense. Currently, all the environment name used for Theorem & Proof in bookdown cannot be used with fenced div for another type of content.

More general thoughts

If we want to allow to support both, we need to add an opt out mechanism for the special environment handling in bookdown, either at the fenced div level or globally.

Globally it could be an option or an argument in output format function that would deactivate the use of the Lua filter. Currently it is not possible to opt-out.

Locally, it could be the addition of a class or an attribute that the Lua filter would check in the div and if present in the div would do nothing.

::: { .solution .preserved}
This is the solution to the problem mentioned above
:::

or

::: { .solution data-bookdown=false}
This is the solution to the problem mentioned above
:::

<div class='solution' data-bookdown=false>
<button>Solution</button>
</div>

We could also revert completely the feature and make it opt-in in bookdown : if one wants to use special Theorem and Proof env, it requires to activate it either globally or locally on each div (like the idea above). Maybe something like

```{r setup, include = FALSE}
bookdown::use_theorems_proofs_env()

could be an API to aim at to activate the usage of a Lua filter. (like it could work for some HTML dependencies)

Opt-in mechanism would definitly prevent this clash of environment name because of this special **bookdown** handling. I guess it may be too late for doing this.

Either case, I think there should be a mechanism to (de)activate this feature in a project if desired. Doing it locally and globally offer great flexibility.