r-lib / downlit

Syntax Highlighting and Automatic Linking
https://downlit.r-lib.org
Other
90 stars 22 forks source link

Can we do any better with multiple identical aliases within 1 package? #167

Closed DavisVaughan closed 1 year ago

DavisVaughan commented 1 year ago

In dplyr 1.1.0 we have dplyr::between() as a function but we also have join_by(between(...)) where between() is part of the DSL for joins but isn't really a function.

In https://github.com/tidyverse/dplyr/pull/6661 we have added aliases so the rest of the join DSL helpers link to the join_by() page. This worked fairly well (even with within() which conflicts with base::within(). It seems like the package alias is preferred over the base one in pkgdown docs, which is nice).

It doesn't work very well for between() though. If you try and add an alias so between links to join_by() then it doesn't do anything. It still links to dplyr::between() instead, which results in the links you see in the images on that PR.

This is an odd scenario, and I'm not sure if we can actually do better, but it might be worth thinking about?

DavisVaughan commented 1 year ago

Part of this is a fundamental limitation of ?between in base R.

In between.Rd we have:

\name{between}
\alias{between}

In join_by.Rd if we add @aliases between then we'd get:

\name{join_by}
\alias{join_by}
\alias{closest}
\alias{overlaps}
\alias{within}
\alias{between}

?between in RStudio always takes me to dplyr::between() even though the same alias exists in both pages.

dmurdoch commented 1 year ago

Aliases are supposed to be unique to a single topic within a package in R. I don't see this documented anywhere, but R CMD check gives a warning if you violate it.

Probably the best solution here is to choose one place to document between, and put an explicit link there to the other page, e.g. in between.Rd say "For use in the DSL, see \code{\link{join_by}}".

You could put \concept{between} in both topics, but you need to use ?? or help.search() to look at concepts, and I don't think many people do that.

DavisVaughan commented 1 year ago

@dmurdoch that is actually our current solution! https://github.com/tidyverse/dplyr/pull/6661/files#diff-b28120b1f9afe7edc7d5501f20ea5945d43c8bcdbe8d0ab5f37cbf35026ec16eR18

This is definitely a reasonable restriction and is a fairly rare situation anyways