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

\item{[}{...} translated incorrectly in Markdown mode #555

Closed
krlmlr opened this issue Jan 4, 2017 · 10 comments
Closed

\item{[}{...} translated incorrectly in Markdown mode #555

krlmlr opened this issue Jan 4, 2017 · 10 comments

Comments

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Jan 4, 2017

tidyverse/tibble@0c4a581

@gaborcsardi
Copy link
Collaborator

@gaborcsardi gaborcsardi commented Jan 4, 2017

@hadley I don't think this needs to be fixed before the release.

@gaborcsardi
Copy link
Collaborator

@gaborcsardi gaborcsardi commented Jan 5, 2017

Actually, this seems to be fine for me:

roc <- rd_roclet()
out1 <- roc_proc_text(roc, "
    #' Title
    #'
    #' Description.
    #'
    #' \\describe{
    #' \\item{`[`}{This.}
    #' \\item{`[[`}{That.}
    #' }
    #'
    #' @md
    foo <- function() {}")[[1]]
out2 <- roc_proc_text(roc, "
    #' Title
    #'
    #' Description.
    #'
    #' \\describe{
    #' \\item{\\code{[}}{This.}
    #' \\item{\\code{[[}}{That.}
    #' }
    #'
    foo <- function() {}")[[1]]
expect_equivalent_rd(out1, out2)

Do you have a self-contained example that fails?

@krlmlr
Copy link
Member Author

@krlmlr krlmlr commented Jan 5, 2017

Adding a simple link breaks it:

roc <- rd_roclet()
out1 <- roc_proc_text(roc, "
    #' Title
    #'
    #' Description.
    #'
    #' \\describe{
    #' \\item{`[`}{This.}
    #' \\item{`[[`}{That. [subset()]. }
    #' }
    #'
    #' @md
    foo <- function() {}")[[1]]
out1

Output:

% Generated by roxygen2: do not edit by hand
% Please edit documentation in RtmpLVDp8r/file173c4115aa40
\name{foo}
\alias{foo}
\title{Title}
\usage{
foo()
}
\description{
Description.
}
\details{
\describe{ \item{\code{[}}{This.} \item{\code{[[}}
{That. [subset()]. } } [\code{}{This.} \item{}[[`}{That.
[subset()]: R:%60%7D%7BThis.%7D%0A%5Citem%7B%60[[%60%7D
%7BThat.%20[subset()
}

@gaborcsardi
Copy link
Collaborator

@gaborcsardi gaborcsardi commented Jan 5, 2017

This looks like a bug indeed.

@hadley
Copy link
Member

@hadley hadley commented Mar 4, 2017

I see a possibly related problem with

#' @param .init If supplied, will be used as the first value to start
#'   the accumulation, rather than using `x[[1]]`. This is useful if
#'   you want to ensure that `reduce` returns the correct value when
#'   `.x` is [is_empty()].

which generates

\item{.init}{If supplied, will be used as the first value to start
the accumulation, rather than using \code{x[[1]]}. This is useful if
you want to ensure that \code{reduce} returns the correct value when
\code{.x} is [is_empty()].

[[1]: R:[1
[is_empty()]: R:is_empty()}

@hadley
Copy link
Member

@hadley hadley commented Aug 21, 2017

Slightly more minimal reprex:

out <- roc_proc_text(rd_roclet(), "
  #' Title
  #'
  #' `[[`. [subset()]
  #'
  #' @md
  foo <- function() {}")[[1]]

out$get_field("description")

@hadley
Copy link
Member

@hadley hadley commented Aug 21, 2017

Actually a minimal reprex:

add_linkrefs_to_md("`[[`. [subset()]")
#> [1] "`[[`. [subset()]\n\n[[`. [subset()]: R:[%60.%20[subset()\n"

@hadley
Copy link
Member

@hadley hadley commented Aug 21, 2017

The problem appears to be that the regexp is matching too much - from the first [.

I think the fix is to also disallow matching [ in the contents of the link - i.e. change [^]] to [^]\\[]. That gives me:

cat(add_linkrefs_to_md("`[[`. [subset()]"))
#> `[[`. [subset()]
#> 
#> [subset()]: R:subset()

@hadley
Copy link
Member

@hadley hadley commented Aug 21, 2017

That gives me this regexp:

regex("(?<=[^]]|^)\\[([^\\]\\[]+)\\](?:\\[([^\\]\\[]+)\\])?(?=[^\\[]|$)")

That doesn't break any unit tests, but I don't understand it fully, so I'll wait for approval from @gaborcsardi before changing.

@brodieG
Copy link
Contributor

@brodieG brodieG commented Sep 6, 2017

Similar issue using 6.0.1.9000. I only list it here for completeness for tests since these don't actually involve links, and in my use case I have the workaround of switching the order of the bullets.

#' This Fails
#'
#' @param bounds character(1L)
#'   * "[)"
#'   * "(]"

test_fun <- function(bounds) TRUE

#' This Works
#'
#' @param bounds character(1L)
#'   * "(]"
#'   * "[)"

test_fun2 <- function(bounds) TRUE

DESCRIPTION:

...
RoxygenNote: 6.0.1.9000
Roxygen: list(markdown = TRUE)

Also, this is what the failing rd looks like:

% Generated by roxygen2: do not edit by hand
% Please edit documentation in R/test.R
\name{test_fun}
\alias{test_fun}
\title{This Fails}
\usage{
test_fun(bounds)
}
\arguments{
\item{bounds}{character(1L)
\itemize{
\item "[)"
\item "(]"
}

[)"
\itemize{
\item "(]: R:)%22%0A%20%20*%20%22(
}}
}
\description{
This Fails
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants