Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

double quotes in restructuredText figure captions erroneously being converted to html #2023

Closed
3 tasks done
trevorld opened this issue Jul 7, 2021 · 7 comments · Fixed by #2269
Closed
3 tasks done
Labels
bug Bugs next Issues/PRs considered for the next release

Comments

@trevorld
Copy link
Contributor

trevorld commented Jul 7, 2021

I've recently noticed in my knitted README and blog posts that double quoted figure captions in restructuredText documents are now being erroneously converted to html in recent versions of {knitr}. {knitr} didn't used to do that so this seems to be new {knitr} bug introduced sometime "recently".

Contents of minimal example test.Rrst:

A simple restructuredText example
=================================

.. {r plot, fig.cap='A figure caption with \'single\' or "double" quotes'}
plot(1:5, rnorm(5))
.. ..

when knit by knitr::knit("test.Rrst") becomes:

A simple restructuredText example
=================================


.. sourcecode:: r
    

    plot(1:5, rnorm(5))

.. figure:: figure/plot-1.png
    :alt: A figure caption with 'single' or "double" quotes

    A figure caption with 'single' or "double" quotes

Instead of "double" we should instead see "double" in both places it appears in the knit .rst document.

Thanks!

xfun::session_info()
R version 4.1.0 (2021-05-18)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 20.04.2 LTS

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

Package version:
  askpass_1.1       brew_1.0.6        brio_1.1.2        cachem_1.0.5     
  callr_3.7.0       cli_3.0.0         clipr_0.7.1       commonmark_1.7   
  compiler_4.1.0    crayon_1.4.1      credentials_1.3.0 curl_4.3.2       
  desc_1.3.0        devtools_2.4.1    diffobj_0.3.4     digest_0.6.27    
  ellipsis_0.3.2    evaluate_0.14     fansi_0.5.0       fastmap_1.1.0    
  fs_1.5.0          gert_1.3.0        gh_1.3.0          gitcreds_0.1.1   
  glue_1.4.2        graphics_4.1.0    grDevices_4.1.0   highr_0.9        
  httr_1.4.2        ini_0.3.1         jsonlite_1.7.2    knitr_1.33.7     
  lifecycle_1.0.0   magrittr_2.0.1    markdown_1.1      memoise_2.0.0    
  methods_4.1.0     mime_0.11         openssl_1.4.4     parallel_4.1.0   
  pillar_1.6.1      pkgbuild_1.2.0    pkgconfig_2.0.3   pkgload_1.2.1    
  praise_1.0.0      prettyunits_1.1.1 processx_3.5.2    ps_1.6.0         
  purrr_0.3.4       R6_2.5.0          rappdirs_0.3.3    rcmdcheck_1.3.3  
  Rcpp_1.0.6        rematch2_2.1.2    remotes_2.3.0     rlang_0.4.11     
  roxygen2_7.1.1    rprojroot_2.0.2   rstudioapi_0.13   rversions_2.1.0  
  sessioninfo_1.1.1 stats_4.1.0       stringi_1.6.2     stringr_1.4.0    
  sys_3.4           testthat_3.0.3    tibble_3.1.2      tools_4.1.0      
  usethis_2.0.1     utf8_1.2.1        utils_4.1.0       vctrs_0.3.8      
  waldo_0.2.5       whisker_0.4       withr_2.4.2       xfun_0.24        
  xml2_1.3.2        xopen_1.0.0       yaml_2.2.1        zip_2.1.1    

By filing an issue to this repo, I promise that

  • I have fully read the issue guide at https://yihui.org/issue/.
  • I have provided the necessary information about my issue.
    • If I'm filing a bug report, I have included a minimal, self-contained, and reproducible example, and have also included xfun::session_info('knitr'). I have upgraded all my packages to their latest versions (e.g., R, RStudio, and R packages), and also tried the development version: remotes::install_github('yihui/knitr').
  • I have learned the Github Markdown syntax, and formatted my issue correctly.

I understand that my issue may be closed if I don't fulfill my promises.

@cderv
Copy link
Collaborator

cderv commented Jul 8, 2021

Thanks for the report.

A git bisect leads me to the issue being introduced by 17a64ff (#1900)

This PR touches fig.alt but for HTML output mainly. Something must have introduced the regression. I'll look into this.

@cderv cderv added bug Bugs next Issues/PRs considered for the next release labels Jul 8, 2021
@cderv
Copy link
Collaborator

cderv commented Jul 8, 2021

And issue is from here:

if (alt) return(escape_html(options$fig.alt %n% cap))

We should escape really only when the output format supports it.

@cderv
Copy link
Collaborator

cderv commented Jul 8, 2021

However, this happens because the caption for rst output uses .img.cap(..., alt = "TRUE")
Maybe there is something to change in how it is handled for rst format

knitr/R/hooks-rst.R

Lines 5 to 19 in 3237add

hook_plot_rst = function(x, options) {
if (options$fig.show == 'animate') return(hook_plot_html(x, options))
cap = .img.cap(options, alt = TRUE)
# TODO: add all options for figure
# See http://docutils.sourceforge.net/docs/ref/rst/directives.html#image
# http://docutils.sourceforge.net/docs/ref/rst/directives.html#figure
make_directive(
'figure',
paste0(opts_knit$get('base.url'), .upload.url(x)),
c(align = if (options$fig.align == 'default') NULL else options$fig.align,
alt = cap, width = options$out.width, height = options$out.height),
cap
)
}

I don't know a lot about asciidoc yet. Is there a way to add alt text for figures ?

@trevorld
Copy link
Contributor Author

trevorld commented Jul 8, 2021

I don't know a lot about asciidoc yet. Is there a way to add alt text for figures ?

I've never used asciidoc but the following documentation says to add an alt text then simply put it in between the square brackets of the image macro: https://docs.asciidoctor.org/asciidoc/latest/macros/images/

@cderv
Copy link
Collaborator

cderv commented Jul 8, 2021

Sorry I just for mixed up. rst is for restructuredText not asciidoc. This is another hook for asciidoc (hook_plot_asciidoc, here we are talking about hook_plot_rst() above)

I see how I can make rst support the new fig.alt option because it should supports it I guess as you can separated :alt: from caption

@trevorld
Copy link
Contributor Author

trevorld commented Jul 8, 2021

I see how I can make rst support the new fig.alt option because it should supports it I guess as you can separated :alt: from caption

Correct. In a rst "figure" directive you can specify a separate :alt: option from the caption paragraph. Here is the official rst "figure" documentation: https://docutils.sourceforge.io/docs/ref/rst/directives.html#figure e.g.

.. figure:: figure/plot-1.png
    :alt: This is the figure "alt" text option

    This is the figure caption (paragraph)

@yihui yihui moved this to To discuss / To plan in R Markdown Team Projects Aug 17, 2023
yihui added a commit that referenced this issue Aug 17, 2023
…alt text can be independently specified via the fig.alt option (#2269)

Co-authored-by: Yihui Xie <xie@yihui.name>
@github-project-automation github-project-automation bot moved this from To discuss / To plan to Done in R Markdown Team Projects Aug 17, 2023
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Bugs next Issues/PRs considered for the next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants