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

Regression: with bookdown 0.22 example (exm) environment misses the colon (:) between Example and its number in odt, docx and epub #1202

Open N0rbert opened 3 years ago

N0rbert commented 3 years ago

I have the following minimal Rmd bookdown example file named index.Rmd:

---
documentclass: book
site: bookdown::bookdown_site
output:
  bookdown::odt_document2: default
  bookdown::epub_book: default
  bookdown::word_document2: default
---

# Example code listing

See Example \@ref(exm:c-code).

::: {custom-style="ListingCaption"}
```{example, c-code}
Simple C program
```
:::
```c
int main()
{
    exit(0);
}
```

Text after example.

With 0.21 it was rendered as "Example 1.1: Simple C program", but with 0.22 and latest version from github it renders as "Example 1.1 Simple C program". It breaks readability. See image below for odt-output:

problem

I think that it is a regression bug.

Below is the table with Example separators comparison.

Version docx odt epub
0.21 : : :
0.22+ no no no

Please confirm and fix this bug in the next version.


> xfun::session_info('bookdown')
R version 3.4.4 (2018-03-15)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 18.04.5 LTS, RStudio 1.4.1717

Locale:
  LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C               LC_TIME=ru_RU.UTF-8       
  LC_COLLATE=en_US.UTF-8     LC_MONETARY=ru_RU.UTF-8    LC_MESSAGES=en_US.UTF-8   
  LC_PAPER=ru_RU.UTF-8       LC_NAME=C                  LC_ADDRESS=C              
  LC_TELEPHONE=C             LC_MEASUREMENT=ru_RU.UTF-8 LC_IDENTIFICATION=C       

Package version:
  base64enc_0.1.3   bookdown_0.22.11  digest_0.6.27     evaluate_0.14    
  glue_1.4.2        graphics_3.4.4    grDevices_3.4.4   highr_0.9        
  htmltools_0.5.1.1 jsonlite_1.7.2    knitr_1.33        magrittr_2.0.1   
  markdown_1.1      methods_3.4.4     mime_0.11         rlang_0.4.11     
  rmarkdown_2.9     stats_3.4.4       stringi_1.6.2     stringr_1.4.0    
  tinytex_0.32      tools_3.4.4       utils_3.4.4       xfun_0.24        
  yaml_2.2.1       
cderv commented 3 years ago

Thanks for the report @N0rbert

I have look into where it comes from and the culprit is #1120

Sorry we missed out this and caused a regression.

I'll fix that quickly

cderv commented 3 years ago

I now remember where this change came from. The : was not supposed to show for any of the theorem environment

bookdown:::theorem_abbr
#>     theorem       lemma   corollary proposition  conjecture  definition 
#>       "thm"       "lem"       "cor"       "prp"       "cnj"       "def" 
#>     example    exercise  hypothesis 
#>       "exm"       "exr"       "hyp"

It was an issue in the code initially that was "fixed" during this PR : https://github.com/rstudio/bookdown/pull/1120/files#r605115658

A <span> should have been included in the first place, and the : sep was unset.

I understand this is a breaking change to you.

@yihui do you remember why a : sep was unset specifically for theorem env ? See change in https://github.com/rstudio/bookdown/pull/1120/files#r605115658 I wonder if we could just use : for non HTML/PDF output in order to not change older output ?

cderv commented 3 years ago

We got here another case where putting back the : broke something: https://community.rstudio.com/t/how-to-change-the-figure-table-caption-style-in-bookdown/110397/2

yihui commented 3 years ago

do you remember why a : sep was unset specifically for theorem env ?

@cderv I think it is uncommon to use : in theorem/proof environments: https://bookdown.org/yihui/bookdown/markdown-extensions-by-bookdown.html#theorems In LaTeX/PDF and HTML output, the environment name and number are in bold text. In other output formats, they do not have any special styling, which is probably why @N0rbert said it affected readability. If we can make them bold like PDF and HTML output, I guess it should be much better and more consistent; otherwise we may have to use the following idea of yours as the last resort.

I wonder if we could just use : for non HTML/PDF output in order to not change older output ?

cderv commented 3 years ago

If we can make them bold like PDF and HTML output,

I think we can do that using markdown syntax by adding ** around the label only for those environment.

diff --git a/R/ebook.R b/R/ebook.R
index 4f38b42e..22974f75 100644
--- a/R/ebook.R
+++ b/R/ebook.R
@@ -116,13 +116,14 @@ resolve_refs_md = function(content, ref_table, to_md = output_md()) {
     for (j in ids) {
       m = sprintf('\\(\\\\#%s\\)', j)
       if (grepl(m, content[i])) {
-        id = ''; sep = ':'
+        id = ''; sep = ':'; bold = FALSE
         type = gsub('^([^:]+).*$', '\\1', j)
         if (type %in% theorem_abbr) {
           id = sprintf('<span id="%s"></span>', j)
           sep = ''
+          bold = TRUE
         }
-        label = label_prefix(type, sep = sep)(ref_table[j])
+        label = label_prefix(type, sep = sep, bold = bold)(ref_table[j])
         content[i] = sub(m, paste0(id, label, ' '), content[i])
         break
       }
diff --git a/R/html.R b/R/html.R
index 6b04a4cd..a422bd6b 100644
--- a/R/html.R
+++ b/R/html.R
@@ -693,7 +693,7 @@ parse_fig_labels = function(content, global = FALSE) {

 # given a label, e.g. fig:foo, figure out the appropriate prefix
-label_prefix = function(type, dict = label_names, sep = '') {
+label_prefix = function(type, dict = label_names, sep = '', bold = FALSE) {
   label = i18n('label', type, dict)
   supported_type = c('fig', 'tab', 'eq')
   if (is.function(label)) {
{
   }
   function(num = NULL) {
     if (is.null(num)) return(label)
-    paste0(label, num, sep)
+    label = paste0(label, num, sep)
+    if (bold) sprintf("**%s**", label) else label
   }
 }

The aim of resolve_refs_md is to replace in the markdown file the reference part by the labels. Then it is process by Pandoc again. So it seems to work as expected.

What do you think ?

cderv commented 3 years ago

If we want to offer a mechanism to customize / opt-out from this, we could extend the mechanism of passing a function also for those environments. We only offer it for figure, table and equation for now: https://github.com/rstudio/bookdown/pull/1120#pullrequestreview-625759445

yihui commented 3 years ago

The PR #1206 looks good to me. @N0rbert What do you think of the bold style? It will look like this:

Example 1.1 Simple C Program

Will that help?

N0rbert commented 3 years ago

If possible, @yihui , please add colon (:) instead of bold style:

Example 1.1: Simple C Program

as it was in previous versions.

yihui commented 3 years ago

I think we can provide an option to add the colon, but by default, I hope not to include the colon, to be consistent with other output formats (PDF and HTML). Will that be okay to you?

N0rbert commented 3 years ago

Providing an option is okay for me. Thanks!

cderv commented 3 years ago

@yihui should it be an option specific for the colon or extending the mechanism to pass a function to customise labels ?

With this mechanism I believe it could be customized any way a user wants it to be.

yihui commented 3 years ago

should it be an option specific for the colon or extending the mechanism to pass a function to customise labels ?

The latter (if possible) would be better. Either way is fine to me.

cderv commented 3 years ago

I think I can extend the mechanism I put in place for fig, tab and eq for now. Otherwise, I'll add an option.