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

Inherit group params exactly #1316

Merged
merged 3 commits into from
Apr 1, 2022
Merged

Inherit group params exactly #1316

merged 3 commits into from
Apr 1, 2022

Conversation

hadley
Copy link
Member

@hadley hadley commented Apr 1, 2022

Fixes #950

Example with ggplot2 at tidyverse/ggplot2#4791

@hadley hadley requested a review from DavisVaughan April 1, 2022 13:29
Copy link
Member

@DavisVaughan DavisVaughan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand all the technical bits, but it overall looks great! I ran this on iv too and it worked well DavisVaughan/ivs#12

R/rd-inherit.R Outdated
@@ -174,8 +176,12 @@ inherit_dot_params <- function(topic, topics, env) {
docs <- lapply(inheritors$source, find_params, topics = topics)
arg_matches <- function(args, docs) {
doc_args <- str_split(names(docs), ", ?")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need doc_args anymore?

#' B
#'
#' @inheritParams A
B <- function(x, y) {}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should you add a test where the args here are .x and .y to ensure that works? I tested it and it does work right, but a test may keep it from breaking in the future

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catches, thanks

@hadley hadley merged commit 0c16c8f into main Apr 1, 2022
@hadley hadley deleted the inherit-grouped-params branch April 1, 2022 19:54
yihui added a commit to rstudio/rmarkdown that referenced this pull request Aug 16, 2022
…ent_base, otherwise the latter can't inherit it from the former because of the change in roxygen2 7.2.0: r-lib/roxygen2#1316
clrpackages pushed a commit to clearlinux-pkgs/R-rmarkdown that referenced this pull request Aug 17, 2022
…n 2.15

Carson Sievert (1):
      Fix html_document's client-side navbar updating in Bootstrap 5 (#2384)

Christophe Dervieux (2):
      Add support for native math by default in Github document (#2362)
      Test pandoc-devel on ubuntu latest (#2398)

Garrick Aden-Buie (1):
      Use `while` instead of `for` loop when adjusting pandoc styles (#2380)

Maëlle Salmon (1):
      Fix typo (#2392)

Michael Chirico (1):
      fix typos (#2386)

Yihui Xie (4):
      mark tufte_handout() as defunct
      use tufte::tufte_handout in test, and remove tufte_handout in doc
      moving the doc of extra_dependencies from html_document to html_document_base, otherwise the latter can't inherit it from the former because of the change in roxygen2 7.2.0: r-lib/roxygen2#1316
      CRAN release v2.15
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.

Keep argument descriptions together when inheriting
2 participants