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

Incorrect rendering of comparison generics <, >, etc. #2303

Closed
bnprks opened this issue May 3, 2023 · 5 comments · Fixed by #2622
Closed

Incorrect rendering of comparison generics <, >, etc. #2303

bnprks opened this issue May 3, 2023 · 5 comments · Fixed by #2622
Labels
bug an unexpected problem or unintended behavior
Milestone

Comments

@bnprks
Copy link

bnprks commented May 3, 2023

The usage section of the html docs for S4 method implementations of comparison generics incorrectly renders < and > with &lt; and &gt;

Minimal reproducible package example here: https://github.com/bnprks/pkgdownbug (incorrectly rendered reference here)

An R file with text:

#' Comparison methods
#'
#' Generic methods and built-in functions for MyType objects
#'
#' @name ComparisonMethods
#' @rdname ComparisonMethods
NULL

setClass("MyType")

#' @describeIn ComparisonMethods Less than comparison
setMethod("<", signature(e1= "MyType", e2= "numeric"), function(e1, e2) {return(TRUE)})
#' @describeIn ComparisonMethods Less than comparison
setMethod("<=", signature(e1= "MyType", e2= "numeric"), function(e1, e2) {return(TRUE)})
#' @describeIn ComparisonMethods Less than comparison
setMethod(">", signature(e1= "MyType", e2= "numeric"), function(e1, e2) {return(TRUE)})
#' @describeIn ComparisonMethods Less than comparison
setMethod(">=", signature(e1= "MyType", e2= "numeric"), function(e1, e2) {return(TRUE)})

gets a usage section rendered to html as follows:

# S4 method for MyType,numeric
&lt;(e1, e2)

# S4 method for MyType,numeric
&lt;=(e1, e2)

# S4 method for MyType,numeric
&gt;(e1, e2)

# S4 method for MyType,numeric
&gt;=(e1, e2)
@maelle maelle added the bug an unexpected problem or unintended behavior label May 4, 2023
@maelle
Copy link
Collaborator

maelle commented May 4, 2023

Thanks for reporting! I don't have time to investigate right now but it looks related to #2286

@hadley hadley added this to the 2.1.0 milestone Apr 12, 2024
@hadley
Copy link
Member

hadley commented Apr 18, 2024

It's already using {{{ }}} in the template, so it must be getting escaped elsewhere 🤔

@hadley
Copy link
Member

hadley commented Apr 18, 2024

Ok, at least two problems here:

  • The escape_html() in the fallback for highlight_text() is (I think) generally unnecessary because flatten_text() calls as_html() which already escapes text (will need to double check this)
  • We're only hitting that fallback because we're generating invalid R code because we're not handling methods for infix functions correctly.

And there's yet another bug in the way we're detecting whether or not a function is infix.

@hadley
Copy link
Member

hadley commented Apr 22, 2024

Ok, the root problem is that the Rd looks like this:

\S4method{<}{MyType,numeric}(e1, e2)

and we want to generate a usage like this:

# S4 method for MyType,numeric
e1 < e2

In particular, note that (e1, e2) is not the call to \S4method{} so we can't the existing Rd tokenization to help us solve this problem.

It looks like base R effectively uses a hand-rolled tokeniser to solve this problem so we'll need to implement something similar in pkgdown.


A couple of test cases for when I take a stab at this:

test_that("infix methods parsed correctly", {
  out <- rd2html("\\S3method{<}{foo}(x, y)")
  expect_equal(out[[2]], "x &lt; y")

  out <- rd2html("\\S4method{<}{foo}(x, y)")
  expect_equal(out[[2]], "x &lt; y")
})

@hadley hadley changed the title Incorrect redering of comparison generics <, >, etc. Incorrect rendering of comparison generics <, >, etc. Apr 25, 2024
hadley added a commit that referenced this issue Jun 3, 2024
hadley added a commit that referenced this issue Jun 3, 2024
hadley added a commit that referenced this issue Jun 3, 2024
@hadley
Copy link
Member

hadley commented Jun 3, 2024

After PR motivating example looks like this:
Screenshot 2024-06-03 at 09 59 40

hadley added a commit that referenced this issue Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants