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

Deduplicate entries in reference index #2302

Merged
merged 2 commits into from
May 9, 2023

Conversation

klmr
Copy link
Contributor

@klmr klmr commented May 2, 2023

When a single documentation entry (say, a function) contains multiple @usage entries with the same function name, this leads to duplicate entries in the ‘pkgdown’ reference entry.

A concrete use-case can be found in the ‘box’ package:

#' …
#' @usage \special{box::file(\dots)}
#' @usage \special{box::file(\dots, module)}
#' …
#' @export
file = function (..., module) {…}

Currently this is rendered as follows:

Screenshot 2023-05-02 at 22 58 23

This PR removes redundant entries in the rendered reference index and adds a relevant test case. I have also added a test case for the similar case of an S3 method that is documented on the same page as its generic. This use-case is currently not affected by the duplicate-entry behaviour but I found it prudent to proactively add a regression test.

Admittedly this is an extreme niche case. However, there’s some precedent for this \usage usage in base R, notably in the documentation of the : operator:

\usage{
from:to
   a:b
}

… as well as the documentation of the [ extract operator.

@hadley
Copy link
Member

hadley commented May 8, 2023

R-exts says:

To indicate that a function can be used in several different ways, depending on the named arguments specified, use section \details.

This is pretty niche feature and I don't think : and [ present a particularly strong argument because they're such old and special functions.

@klmr
Copy link
Contributor Author

klmr commented May 8, 2023

R-exts says:

To indicate that a function can be used in several different ways […] use section \details.

That is correct, but in my (yes, admittedly niche) use-case it would be vastly less clear, in particular for the documentation of box::use:

box::use(prefix/mod, ...)
box::use(pkg, ...)
box::use(alias = prefix/mod, ...)
box::use(alias = pkg, ...)
box::use(prefix/mod[attach_list], ...)
box::use(pkg[attach_list], ...)

Deferring all this into the “Details” section and making the documentation less easily understood, for no reason other than to satisfy a rigid rule, prioritises dogmatism over user friendliness.

Anyway, I understand not wanting to maintain a feature that few if any people besides me would ever use. I can’t even claim that this PR is a bug fix. Unfortunately without this change I have to maintain a fork of this package plus a custom CI/CD pipeline to build the documentation. And the PR amounts to a single unique() function call (+ tests), so I was hoping adding it here would add negligible maintenance overhead.

@hadley
Copy link
Member

hadley commented May 8, 2023

I think the main thing that bugs me about this PR is the impact on all the tests. What if you just dropped them?

@klmr klmr force-pushed the fix/deduplicate-ref-entries branch from 2ef1808 to 13bdf5c Compare May 9, 2023 19:31
@klmr
Copy link
Contributor Author

klmr commented May 9, 2023

😆

… Fair enough, I see the point. Done.

@hadley
Copy link
Member

hadley commented May 9, 2023

Can you please rebase against main too?

@klmr klmr force-pushed the fix/deduplicate-ref-entries branch from 13bdf5c to 5ef3ae4 Compare May 9, 2023 21:04
@hadley hadley merged commit 44c92aa into r-lib:main May 9, 2023
@hadley
Copy link
Member

hadley commented May 9, 2023

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants