Skip to content

Basic support for S7#1843

Merged
hadley merged 13 commits intomainfrom
s7
Apr 7, 2026
Merged

Basic support for S7#1843
hadley merged 13 commits intomainfrom
s7

Conversation

@hadley
Copy link
Copy Markdown
Member

@hadley hadley commented Mar 31, 2026

Fixes #1484

@hadley hadley requested a review from t-kalinowski March 31, 2026 17:54
Copy link
Copy Markdown
Member

@t-kalinowski t-kalinowski left a comment

Choose a reason for hiding this comment

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

Looks good to me. I tried it out in ellmer with a few variations.

One other note: the @prop syntax is nice, but on Rd pages where many classes are documented together, there is currently no way to tie a property back to its owner. For example, ?ellmer::Content currently documents 9 classes together. If you add @prop foo foo to one of the subclasses, the rendered page has a flat Additional properties section, but it is impossible to tell which class the @foo property belongs to.

Comment thread vignettes/rd-other.Rmd
#' @param x A `Range` object.
#' @param ... Not used.
#' @returns A single number.
method(size, Range) <- function(x, ...) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is really nice how it "just works" without any special syntax!

Comment thread vignettes/rd-other.Rmd
#' @export
size <- new_generic("size", "x")

#' @rdname size
Copy link
Copy Markdown
Member

@t-kalinowski t-kalinowski Apr 6, 2026

Choose a reason for hiding this comment

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

What do you think about making generation of docs for registered methods 'opt-out' with @nord rather than 'opt-in' with @rdname size?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Unfortunately I think that ship has sailed, because I think we want to be consistent with S3, and changing that would affect a lot of existing code.

@hadley
Copy link
Copy Markdown
Member Author

hadley commented Apr 7, 2026

I like the idea of being able to specify multiple properties in the same Rd topic, but there's currently no way obvious way to pass the class name into the correct tag object. So I made it possible for the user to supply with @prop class@propname description. That feels like a reasaonable compromise to me.

@hadley hadley merged commit 664a828 into main Apr 7, 2026
13 checks passed
@hadley hadley deleted the s7 branch April 7, 2026 22:23
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.

Add support for S7

2 participants