rstudio / bookdown

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

don't respect beamer theme's buildin theorem/proof block #1143

Closed XiangyunHuang closed 3 years ago

XiangyunHuang commented 3 years ago

here is a minimal, self-contained, and reproducible example

---
title: "beamer"
documentclass: beamer
output: 
  bookdown::pdf_book: 
    base_format: rmarkdown::beamer_presentation
    toc: no
    latex_engine: xelatex
    citation_package: natbib
    theme: Xiaoshan # https://www.ctan.org/pkg/pgfornament-han 
    template: null
link-citations: yes
colorlinks: yes
---

```{r setup, include=FALSE}
knitr::opts_chunk$set(echo = FALSE)

some blocks from beamer theme Xiaoshan

::: {.block data-latex="{Metropolis}"} block is ok. :::

::: {.exampleblock data-latex="{Metropolis}"} exampleblock is also ok. :::

::: {.theorem data-latex="{Metropolis}"} theorem is not ok. :::


my session info

```r
xfun::session_info('bookdown')
R version 4.0.4 (2021-02-15)
Platform: x86_64-apple-darwin20.3.0 (64-bit)
Running under: macOS Big Sur 10.16, RStudio 1.4.1106

Locale: en_US.UTF-8 / en_US.UTF-8 / en_US.UTF-8 / C / en_US.UTF-8 / en_US.UTF-8

Package version:
  base64enc_0.1.3   bookdown_0.21     digest_0.6.27     evaluate_0.14     glue_1.4.2       
  graphics_4.0.4    grDevices_4.0.4   highr_0.9         htmltools_0.5.1.1 jsonlite_1.7.2   
  knitr_1.32        magrittr_2.0.1    markdown_1.1      methods_4.0.4     mime_0.10        
  rlang_0.4.10      rmarkdown_2.7     stats_4.0.4       stringi_1.5.4     stringr_1.4.0    
  tinytex_0.31      tools_4.0.4       utils_4.0.4       xfun_0.22         yaml_2.2.1      

This bug also reported here

cderv commented 3 years ago

Oh yes I see what is the issue.

I think this message is printed in the console during R Markdown rendering:

[WARNING] data-latex attribute can't be used with one of bookdown custom environment. It has been removed.

As bookdown already use a theorem environment built-in, one can't use the custom block syntax from https://bookdown.org/yihui/rmarkdown-cookbook/custom-blocks.html with one of the supported environemnt. We do that to prevent a double processing by rmarkdown specific Lua filter for LaTeX divs (which is used in this example).

Obviously we are not supporting beamer correctly in the Lua filter or at least in our logic. Also, maybe there should be a way to opt-out using bookdown's environment feature and let users customize directly according to its needs.

Is theorem an environment built in beamer ? I don't know much.

cderv commented 3 years ago

Is theorem an environment built in beamer ? I don't know much.

It seems to be at least for Metropolis theme in beamer https://www.tug.org/texlive//Contents/live/texmf-dist/doc/latex/beamertheme-metropolis/metropolistheme.pdf

This will conflict with our mechanism in restore_block2() where we add the definition in preamble if not already. https://github.com/rstudio/bookdown/blob/df6786eb42f34c9737449f4ef9608c8e9cc76932/R/latex.R#L233-L236

If we support beamer the \begin{theorem} will be written in tex file which will trigger the above and add a new definition - which should not be there as beamer already defines one.

We could

The more generic in the second one. πŸ€”

yihui commented 3 years ago

@XiangyunHuang By using the data-latex attribute, did you just want to provide a name to the theorem? If so, the syntax is:

::: {.theorem name="Metropolis"}
theorem is ok.
:::

But we don't support beamer yet. @cderv I don't remember why we didn't support beamer, but I feel we should: https://github.com/rstudio/bookdown/blob/6854e02da9b21b95f88119ed36f38e91a4f9e1b2/inst/rmarkdown/lua/custom-environment.lua#L134

XiangyunHuang commented 3 years ago

@yihui When I use the way you provided, it's not ok, either. I have upgraded bookdown to latest dev version, here is new session info

xfun::session_info('bookdown')
#> R version 4.0.4 (2021-02-15)
#> Platform: x86_64-apple-darwin20.3.0 (64-bit)
#> Running under: macOS Big Sur 11.2.3
#> 
#> Locale: en_US.UTF-8 / en_US.UTF-8 / en_US.UTF-8 / C / en_US.UTF-8 / en_US.UTF-8
#> 
#> Package version:
#>   base64enc_0.1.3   bookdown_0.21.11  digest_0.6.27     evaluate_0.14    
#>   glue_1.4.2        graphics_4.0.4    grDevices_4.0.4   highr_0.9        
#>   htmltools_0.5.1.1 jsonlite_1.7.2    knitr_1.32        magrittr_2.0.1   
#>   markdown_1.1      methods_4.0.4     mime_0.10         rlang_0.4.10     
#>   rmarkdown_2.7     stats_4.0.4       stringi_1.5.4     stringr_1.4.0    
#>   tinytex_0.31      tools_4.0.4       utils_4.0.4       xfun_0.22        
#>   yaml_2.2.1

Created on 2021-04-20 by the reprex package (v2.0.0)

ζˆͺ屏2021-04-20 上午8 48 51
yihui commented 3 years ago

That's expected. As I said above, the theorem Divs don't work for beamer yet. We'll probably fix it.

cderv commented 3 years ago

@yihui we don't support it because the previous syntax does not support either

> dir.create(tmp_dir <- tempfile())
> owd <- setwd(tmp_dir)
> xfun::write_utf8(c(
+ '---
+ title: "Minimal Working Example"
+ output: bookdown::beamer_presentation2
+ ---
+ 
+ ```{theorem}
+ test
+ ```'
+ ), "test.Rmd")
> rmarkdown::render("test.Rmd", "bookdown::beamer_presentation2", quiet = TRUE)
! LaTeX Error: Command \theorem already defined.
               Or name \end... illegal, see p.192 of the manual.

Error: LaTeX failed to compile test.tex. See https://yihui.org/tinytex/r/#debugging for debugging tips. See test.log for more info.
Run `rlang::last_error()` to see where the error occurred.
> setwd(owd)
> unlink(tmp_dir, recursive = TRUE)

To run the above code: Copy then use reprex::reprex_rescue() before pasting in a script

Problem is the same here: beamer already define a theorem environment and currently our internal restore_block2 does not take that into account. I will then add in the preamble a second definition. Hence the issue

! LaTeX Error: Command \theorem already defined.

Fixing this should allow to support the new syntax too.

cderv commented 3 years ago

Looking more into beamer by doing small quick test, it seems that the following among bookdown Theorem and proof env are already defined when using a documentclass beamer

The other are not defined and their definition by bookdown in preamble does not cause error

So we are in a hybrid state with beamer.

Currently what we do when we detect than one of the environment is used:

So we could either do if we detect it is beamer used:

  1. we don't add any new definition. This would mean that missing environment should be defined by the user in a preamble.tex as bookdown won't add any. Also internationalization won't work for those environment.
  2. we only add add the missing environment defintion. internationalization won't work for env define in beamer though.

1 is the simpler and we let the user handle the rest. 2 is the better to support all env documented as supported in bookdown. I wonder if we could modify the already define one πŸ€”

cderv commented 3 years ago

Ok I found the source: https://github.com/josephwright/beamer/blob/56b5d77a5b1ad886f452f23148b0e2e36094c689/base/beamerbasetheorems.sty#L114

This where the \newtheorem happens and translation is done in a better way than in bookdown using \translate command from the translator CTAN package.

So I tend to think that with beamer we do nothing special to let user customize as they need.

cderv commented 3 years ago

Oh and also @XiangyunHuang, I hope you notice that you don't need bookdown for this example to work.

---
title: "beamer"
output: 
  rmarkdown::beamer_presentation:
    toc: no
    latex_engine: xelatex
    citation_package: natbib
    theme: Xiaoshan # https://www.ctan.org/pkg/pgfornament-han 
    template: null
link-citations: yes
colorlinks: yes
---

```{r setup, include=FALSE}
knitr::opts_chunk$set(echo = FALSE)

some blocks from beamer theme Xiaoshan

::: {.block data-latex="{Metropolis}"} block is ok. :::

::: {.exampleblock data-latex="{Metropolis}"} exampleblock is also ok. :::

::: {.theorem data-latex="{Metropolis}"} theorem is not ok. :::



But you may need to use **bookdown** for its other features. (Cross referencies I believe ?)
cderv commented 3 years ago

we don't support it because the previous syntax does not support either

1145 will fix this for now.

XiangyunHuang commented 3 years ago

But you may need to use bookdown for its other features. (Cross referencies I believe ?)

Yes.

cderv commented 3 years ago

@XiangyunHuang just to confirm, what output is expected for this ?

::: {.theorem data-latex="{Metropolis}"}
theorem is not ok.
:::
XiangyunHuang commented 3 years ago

@cderv The expected output is

ζˆͺ屏2021-04-21 上午12 33 00

here is a minimal Xiaoshan beamer demo.

% !TEX program = xelatex
\documentclass{beamer}
\usetheme{Xiaoshan}

\title{Xiaoshan}

\begin{document}

\begin{frame}
  \maketitle
\end{frame}

\begin{frame}
  \frametitle{some blocks from beamer theme Xiaoshan}

  \begin{block}{Metropolis}
   block is ok.
  \end{block}

  \begin{exampleblock}{Metropolis}
   exampleblock is ok.
  \end{exampleblock}

  \begin{alertblock}{Metropolis}
    alertblock is ok.
  \end{alertblock}
\end{frame}

\begin{frame}
  \frametitle{some blocks from beamer theme Xiaoshan}

  \begin{theorem}[Metropolis]
    theorem is ok.
  \end{theorem}

  \begin{proof}[Metropolis]
    proof is also ok.
  \end{proof}
\end{frame}

\end{document}
cderv commented 3 years ago

Thanks. That makes more sense then.You want \begin{theorem}[Metropolis] and not \begin{theorem}{Metropolis}

This means that it should be without bookdown support

::: {.theorem data-latex="[Metropolis]"}
theorem is not ok.
:::

and as @yihui has shown above this would be the bookdown ways if it was supported

::: {.theorem name="Metropolis"}
theorem is ok.
:::

We'll fix this for next version

cderv commented 3 years ago

Can you try the PR now ?

remotes::install_github("rstudio/bookdown#1145")

beamer should be supported now as other format using the same syntax as other format

---
title: "beamer"
output: 
  bookdown::beamer_presentation2:
    toc: no
    latex_engine: xelatex
    citation_package: natbib
    theme: Xiaoshan # https://www.ctan.org/pkg/pgfornament-han 
    template: null
link-citations: yes
colorlinks: yes
---

```{r setup, include=FALSE}
knitr::opts_chunk$set(echo = FALSE)

some blocks from beamer theme Xiaoshan

::: {.block latex="{Metropolis}"} block is ok. :::

::: {.exampleblock latex="{Metropolis}"} exampleblock is also ok. :::

::: {.theorem name="Metropolis"} theorem is now ok. :::


![image](https://user-images.githubusercontent.com/6791940/115444439-da460680-a214-11eb-83cf-1e7dad0b3257.png)

This would allow the referencing to work by adding an id but I am seing that beamer Theorem environment are not numbered. I don't know them so maybe it is expected. 

@yihui @XiangyunHuang what do you think ? 

Other solution is to completely ignore **bookdown** feature for _beamer_ output and just use R Markdown custom block feature for LateX

This would mean that this would work in that case 

````markdown
---
title: "beamer"
output: 
  bookdown::beamer_presentation2:
    toc: no
    latex_engine: xelatex
    citation_package: natbib
    theme: Xiaoshan # https://www.ctan.org/pkg/pgfornament-han 
    template: null
link-citations: yes
colorlinks: yes
---

```{r setup, include=FALSE}
knitr::opts_chunk$set(echo = FALSE)

some blocks from beamer theme Xiaoshan

::: {.block latex="{Metropolis}"} block is ok. :::

::: {.exampleblock latex="{Metropolis}"} exampleblock is also ok. :::

::: {.theorem latex="[Metropolis]"} theorem is now ok. :::



This what you would write when not using **bookdown** but using `rmarkdown::beamer_presentation`
cderv commented 3 years ago

Oh now I see the screenshot here, there is maybe a styling issue with the current solution.

Afterall, I should maybe ignore the Lua filter for beamer presentation then - this could be simpler πŸ€”

Or this may be the sign of an issue in how I build the tex content in the Lua filter πŸ˜“

cderv commented 3 years ago

Ok I know what is going one with the styling. This is because we add a special syntax for linking to theorem

\protect\hypertarget{thm:unlabeled-div-1}{}\label{thm:unlabeled-div-1}

This seems to cause the empty line with beamer. It did no seem to cause anything with other pdf format.

So as I said after all I may just ignore beamer in bookdown and make sure the latex= attributes can be used. Instead of trying to adapt the above only for beamer usage. I'll do that tomorrow and leave the PR as is for you to try if you want.

XiangyunHuang commented 3 years ago

@cderv

remotes::install_github("rstudio/bookdown#1145")

is fine. but I hope we don't change

::: {.block data-latex="{Metropolis}"}
block is ok.
:::

to

::: {.block latex="{Metropolis}"}
block is ok.
:::

the block / exampleblock / theorem be more consistent the better, both beamer and book documentclass.

cderv commented 3 years ago

data-latex and latex are both working - one is an alias to the other.

See PR #1145 for following discussion on the syntax.

github-actions[bot] commented 3 years ago

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue by following the issue guide (https://yihui.org/issue/), and link to this old issue if necessary.