r-lib / roxygen2

Generate R package documentation from inline R comments
https://roxygen2.r-lib.org
Other
594 stars 233 forks source link

Extra { around inherited href tags #778

Closed isteves closed 5 years ago

isteves commented 6 years ago

I've been getting the following error when running devtools::check:

checking Rd files ... WARNING
prepare_Rd: bad markup (extra space?) at test.Rd:11:113
prepare_Rd: bad markup (extra space?) at test.Rd:14:38
prepare_Rd: bad markup (extra space?) at test.Rd:15:48

These lines all contain links that have been inherited via @inheritParams. They work fine in the original package (usethis::create_project(), rstudio argument) but for some reason get an extra { when they're inherited:

\href{https://here.r-lib.org}{here}

versus

\href{{https://here.r-lib.org}{here}} 

I've created a minimal package that isolates the problem. I'm using roxygen version 6.1.0.

isteves commented 6 years ago

Perhaps also relevant: the original roxygen link is written using [link text](link).

#' @param rstudio If `TRUE`, calls [use_rstudio()] to make the new package or
#'   project into an [RStudio
#'   Project](https://support.rstudio.com/hc/en-us/articles/200526207-Using-Projects).
#'    If `FALSE` and a non-package project, a sentinel `.here` file is placed so
#'   that the directory can be recognized as a project by the
#'   [here](https://here.r-lib.org) or
#'   [rprojroot](https://rprojroot.r-lib.org) packages.
hadley commented 6 years ago

Is this the same as #635?

isteves commented 6 years ago

Seems a bit different to me, but perhaps related? They are both similar in that inherit expects one argument. \something{one_arg} is okay but \something{one_arg}{two_args} is not

In 635, the package context should be added in but isn't: \link{world} should be \link[maps:world]{world}

Here, there are extra curly brackets but no extra context is needed: \href{{https://here.r-lib.org}{here}} should be \href{https://here.r-lib.org}{here}

mjsteinbaugh commented 5 years ago

I'm starting to hit this issue pretty frequently when attempting to improve my documentation by adding more links (either \href or \link).

sctyner commented 5 years ago

Same here. Here's my specific use case, reprex'd.

library(roxygen2)
roc_proc_text(rd_roclet(), "
  #' Title
  #'
  #' @inheritDotParams magick::image_annotate location degrees boxcolor
  wrapper <- function(...) magick::image_annotate(...)"
)
#> $wrapper.Rd
#> % Generated by roxygen2: do not edit by hand
#> % Please edit documentation in RtmpeMScoX/filed6a2276933a9
#> \name{wrapper}
#> \alias{wrapper}
#> \title{Title}
#> \usage{
#> wrapper(...)
#> }
#> \arguments{
#>   \item{...}{Arguments passed on to
#>   \code{magick::image_annotate} \describe{ \item{location}
#>   {geometry string with location relative to \code{gravity}}
#>   \item{degrees}{rotates text around center point}
#>   \item{boxcolor}{a \href{{https://www.imagemagick.org/Magick+
#>   +/Color.html}{color string}} for background color that
#>   annotation text is rendered on.} }}
#> }
#> \description{
#> Title
#> }

Created on 2019-04-16 by the reprex package (v0.2.1)

hadley commented 5 years ago

Minimal reprex using internal functions:

get_params <- function(pkg, fun) {
  rd <- roxygen2:::get_rd(fun, pkg)
  roxygen2:::topic_params.Rd(rd)
}

cat(get_params("usethis", "create_project")[["rstudio"]])
#> If \code{TRUE}, calls \code{\link[=use_rstudio]{use_rstudio()}} to make the new package or
#> project into an \href{{https://support.rstudio.com/hc/en-us/articles/200526207-Using-Projects}{RStudio Project}}.
#> If \code{FALSE} and a non-package project, a sentinel \code{.here} file is placed so
#> that the directory can be recognized as a project by the
#> \href{{https://here.r-lib.org}{here}} or
#> \href{{https://rprojroot.r-lib.org}{rprojroot}} packages.
cat(get_params("tidyr", "unnest")[["..."]])
#> Name-variable pairs of the form \code{new_col = c(col1, col2, col3)},
#> that describe how you wish to nest existing columns into new columns.
#> The right hand side can be any expression supported by tidyselect.
#> 
#> \Sexpr[results=rd, stage=render]{lifecycle::badge("deprecated")}: previously you could write \code{df \%>\% nest(x, y, z)}
#> and \code{df \%>\% unnest(x, y, z)}. Convert to \code{df \%>\% nest(data = c(x, y, z))}.
#> and \code{df \%>\% unnest(c(x, y, z))}.
#> 
#> If you previously created new variable in \code{unnest()} you'll now need to
#> do it explicitly with \code{mutate()}. Convert \code{df \%>\% unnest(y = fun(x, y, z))}
#> to \code{df \%>\% mutate(y = fun(x, y, z)) \%>\% unnest(y)}.

Interestingly it's specifically \href{} that's the problem; \code{}, \link{}, and \Sexpr{} are all fine.

hadley commented 5 years ago

Even more minimal reprex:

tools::parse_Rd(textConnection("\\href{a}{b}"), fragment = TRUE)
#> \href{{a}{b}}

Created on 2019-08-13 by the reprex package (v0.3.0)

So this looks like a base R problem with the Rd pretty printer. Is anyone interesting in tracking this down further and filing a bug report with R-core?

andrewmarx commented 5 years ago

I randomly came across this open issue and am exploring it out of sheer curiosity. I may or may not be able to track it down all the way. In case someone else is interested in trying: so far I've traced it to parseRd() in gramRd.c, but this is now getting into lexical analysis which is not something I'm experienced with. One interesting thing is that the keywords[] array in this file has different integers assigned for different possible tokens:

...

/* This macro takes one verbatim and one LaTeX-like argument. */

{ "\\href",    VERBLATEX },

/* This macro takes three LaTeX-like arguments. */

{ "\\ifelse",  LATEXMACRO3 },

/* These macros take one optional bracketed option and always take 
   one LaTeX-like argument */

{ "\\link",    OPTMACRO },

/* These markup macros require an R-like text argument */

{ "\\code",    RCODEMACRO },
{ "\\dontshow",RCODEMACRO },
{ "\\donttest",RCODEMACRO },
{ "\\testonly",RCODEMACRO },

/* This macro takes one optional bracketed option and one R-like argument */

{ "\\Sexpr",   SEXPR },

...

Which might explain why \code{}, \link{}, and \Sexpr{} work; they are handled differently by the parser.

andrewmarx commented 5 years ago

I spent a little more time on it this morning, and am reasonably sure the cause is in yyparse() in gramRd.c. Unfortunately, this function (Bison parser) looks pretty involved and I don't have time to figure out how it works. Whether it is an actual issue in this function (or one of the ones it calls), or if the mapped value in the keywords[] array just needs to be changed, I don't know.

For whoever takes this up, the values in the keywords[] array are being mapped to values in the yytranslate[] array, which are then used in the switch statements in yyparse(). In the case of "\href", the keywords[] value is 277, which gives 22 from yytranslate[]. To get to where I suspect you need to be, look for this spot in gramRd.c:

  case 22:

    { (yyval) = (yyvsp[0]); }
hadley commented 5 years ago

Actually I think the problem might be a bit simpler — as.character.Rd contains a vector called TWOARG and adding \\href seems to fix the problem.