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

feature: Render mixed-in methods and constants with --embed-mixins #842

Merged
merged 4 commits into from
Oct 17, 2024

Conversation

flavorjones
Copy link
Contributor

When --embed-mixins option is set:

  • methods from an extended module are documented as singleton methods
  • attrs from an extended module are documented as class attributes
  • methods from an includeed module are documented as instance methods
  • attrs from an includeed module are documented as instance attributes
  • constants from an includeed module are documented

Sections are created when needed, and Darkfish's template annotates
each of these mixed-in CodeObjects.

This feature is inspired by Yard's option of the same name.


Example class:

module IncMod
  # :section: Things That Were Included

  # incmod attribute
  attr :incmod_attr

  # constant via mixin
  INCMOD_CONST = "incmod_const"

  # instance method via mixin
  def incmod_method
    "incmod_method"
  end
end

module ExtMod
  # :section: Things That Were Extended

  # extmod attribute
  attr :extmod_attr

  # class method via mixin
  def extmod_method
    "extmod_method"
  end
end

class Foo
  include IncMod
  extend ExtMod
end

Currently this renders like:

Screenshot from 2021-09-23 02-01-18

With --embed-mixins it renders like:

Screenshot from 2021-09-23 02-01-38

@flavorjones flavorjones changed the title Embed mixed-in methods and constants with --embed-mixins feature: Render mixed-in methods and constants with --embed-mixins Oct 11, 2021
@flavorjones
Copy link
Contributor Author

I've pushed an update to this PR to ensure that code object visibility is respected (which was a bug in the original patch).

@flavorjones
Copy link
Contributor Author

@aycabta I'm curious if you have feedback on this feature? It's off by default but I think would be a real improvement to standard-library classes like Array that inherit significant functionality from Enumerable.

@flavorjones
Copy link
Contributor Author

@aycabta This feature is live at https://nokogiri.org/rdoc/Nokogiri/XML/Node.html#method-i-at_css if you'd like to see what it looks like. Is this something you would consider merging?

@flavorjones
Copy link
Contributor Author

flavorjones commented Dec 8, 2022

Rebased off v6.5.0.

I still think this is a valuable feature to consider. It's been in use at https://nokogiri.org/rdoc/ for the past year, see https://nokogiri.org/rdoc/Nokogiri/XML/Node.html#method-i-css for example.

@flavorjones
Copy link
Contributor Author

Rebased again off master. @aycabta @nobu would you consider merging this feature?

@flavorjones
Copy link
Contributor Author

I've rebased again onto master. I still think this would be a valuable feature to merge. @hsbt @drbrain I would appreciate any feedback you have for me.

@flavorjones
Copy link
Contributor Author

@hsbt If you get some time to review this, I still think it's a good feature, and it's been live on the nokogiri.org site for several years now.

Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

Left some small suggestions, but overall looks good to me.
How does it affect documentation displayed by ri though?

test/rdoc/test_rdoc_class_module.rb Show resolved Hide resolved
lib/rdoc/class_module.rb Outdated Show resolved Hide resolved
lib/rdoc/code_object.rb Show resolved Hide resolved
@flavorjones
Copy link
Contributor Author

@st0012 Thank you for your review!

How does it affect documentation displayed by ri though?

I hadn't even considered ri as a target, since I don't usually use it. I will explore what's necessary to extend the ri content as well.

@flavorjones
Copy link
Contributor Author

@st0012 So ri is working out of the box with this.

If I generate the ri files with rdoc --embed-mixins --format=ri then the resulting behavior is what I'd expect, which is that it shows the mixed-in methods under all of the classes.

$ ri at_css
= .at_css

(from /home/flavorjones/code/oss/nokogiri/ri)
=== Implementation from Node
------------------------------------------------------------------------
  at_css(*rules, [namespace-bindings, custom-pseudo-class])

------------------------------------------------------------------------

Search this object for CSS rules, and return only the first match. rules
must be one or more CSS selectors.

See Searchable#css for more information.


(from /home/flavorjones/code/oss/nokogiri/ri)
=== Implementation from NodeSet
------------------------------------------------------------------------
  at_css(*rules, [namespace-bindings, custom-pseudo-class])

------------------------------------------------------------------------

Search this object for CSS rules, and return only the first match. rules
must be one or more CSS selectors.

See Searchable#css for more information.


(from /home/flavorjones/code/oss/nokogiri/ri)
=== Implementation from Searchable
------------------------------------------------------------------------
  at_css(*rules, [namespace-bindings, custom-pseudo-class])

------------------------------------------------------------------------

Search this object for CSS rules, and return only the first match. rules
must be one or more CSS selectors.

See Searchable#css for more information.


@st0012
Copy link
Member

st0012 commented Jul 2, 2024

@flavorjones 👋 just want to say that I didn't forget this PR. @colby-swandale and I just got commit bit to this project and we will likely do some refactors/test improvements...etc. with some bug fixes first. And then we'll start merging features.

@flavorjones
Copy link
Contributor Author

No sweat, @st0012! Let me know if I can be helpful with anything.

@flavorjones
Copy link
Contributor Author

Rebased onto current master and force-pushed.

@st0012 st0012 added this to the v6.8.0 milestone Oct 17, 2024
flavorjones and others added 4 commits October 17, 2024 13:37
When `--embed-mixins` option is set:

- methods from an `extend`ed module are documented as singleton methods
- attrs from an `extend`ed module are documented as class attributes
- methods from an `include`ed module are documented as instance methods
- attrs from an `include`ed module are documented as instance attributes
- constants from an `include`ed module are documented

Sections are created when needed, and Darkfish's template annotates
each of these mixed-in CodeObjects. We also respect the mixin methods'
visibility.

This feature is inspired by Yard's option of the same name.
@flavorjones
Copy link
Contributor Author

Rebased onto current master and force-pushed.

Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

Tested with RDoc and I think it works well. Thank you for this great addition 🎉

@st0012 st0012 merged commit 481c2ce into ruby:master Oct 17, 2024
26 checks passed
matzbot pushed a commit to ruby/ruby that referenced this pull request Oct 17, 2024
`--embed-mixins`
(ruby/rdoc#842)

* Embed mixed-in methods and constants with `--embed-mixins`

When `--embed-mixins` option is set:

- methods from an `extend`ed module are documented as singleton methods
- attrs from an `extend`ed module are documented as class attributes
- methods from an `include`ed module are documented as instance methods
- attrs from an `include`ed module are documented as instance attributes
- constants from an `include`ed module are documented

Sections are created when needed, and Darkfish's template annotates
each of these mixed-in CodeObjects. We also respect the mixin methods'
visibility.

This feature is inspired by Yard's option of the same name.

* Add comment to document why we set object visibility

Co-authored-by: Stan Lo <[email protected]>

* Add the mixin_from attribute to CodeObject's initializer

* Add test coverage for private mixed-in attributes.

---------

ruby/rdoc@481c2ce660

Co-authored-by: Stan Lo <[email protected]>
@flavorjones flavorjones deleted the flavorjones/embed-mixins branch October 17, 2024 20:49
@flavorjones
Copy link
Contributor Author

woo hoo! thank you, @st0012!

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

Successfully merging this pull request may close these issues.

3 participants