Skip to content

Fix method source code not created if a method's description is skipped #1488

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.

Sign up for GitHub

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

Merged
tompng merged 2 commits into from
Dec 9, 2025
+6 −10
Merged

Fix method source code not created if a method's description is skipped #1488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
71844a2
[Darkfish] Fix html for method source code not created if a method's …
extern-c Dec 8, 2025
0837c66
[Aliki] Fix html for method source code not created if a method's des…
extern-c Dec 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions lib/rdoc/generator/template/aliki/class.rhtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,13 @@
<summary>Source</summary>
</details>
</div>
<div class="method-source-code" id="<%= method.html_name %>-source">
<pre class="<%= method.source_language %>" data-language="<%= method.source_language %>"><%= method.markup_code %></pre>
</div>
Copy link
Member

@tompng tompng Dec 8, 2025

Choose a reason for hiding this comment

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

Moving this will change this HTML structure:

<div class="method-heading"></div>
<div class="method-controls"></div>
<div class="method-description"></div>

How about moving the whole <%- if method.token_stream %> method controls <% -end %> to

<%- unless method.skip_description? then %>
(HERE)
<div class="method-description">
  ...
</div>
...
<%- end %>

Copy link
Member

@st0012 st0012 Dec 8, 2025

Choose a reason for hiding this comment

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

I'm aware of this but it also feels like source code div actually shouldn't be placed under description, if we want their display to be separate?

Copy link
Member

@tompng tompng Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

if we want their display to be separate

I think there is no need to separate. If there's a reason to skip description, (example: it's an alias and the description/source are both visible in the original method), we don't need to show method source.
This will reduce generated HTML size especially when many aliases are defined. (edited. this part seems wrong)

For Hash#has_value?, it shouldn't skip description. #1491 will fix it from the backend side.

Copy link
Member

@tompng tompng Dec 9, 2025

Choose a reason for hiding this comment

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

Looks like method.skip_description? and method.token_stream.nil? is almost identical (especially after some bug fixes of backend),
I reconsidered it's OK moving method-description outside of <%- unless method.skip_description? then %> 👍

Copy link
Contributor Author

@extern-c extern-c Dec 9, 2025

Choose a reason for hiding this comment

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

Hi @st0012 and @tompng,

Thanks for the code review! For my own understanding, would you mind confirming when method.skip_description? is supposed to return true?

Copy link
Member

@tompng tompng Dec 9, 2025

Choose a reason for hiding this comment

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

when method.skip_description? is supposed to return true?

In the source code document

# Whether to skip the method description, true for methods that have
# aliases with a call-seq that doesn't include the method name.
def skip_description?

Here's an example:

/*
 * call-seq:
 *   foo() -> integer
 *   bar() -> integer
 * Returns 42
 */
VALUE rb_obj_foo(VALUE self){return INT2FIX(42);}
void Init_FooBar() {
  rb_define_method(rb_cObject, "foo", rb_obj_foo);
  rb_define_method(rb_cObject, "bar", rb_obj_foo);
  rb_define_method(rb_cObject, "baz", rb_obj_foo);
}

The same method is defined as foo, bar and baz. But call-seq only have foo and bar.
In this case, description for baz is skipped.

Actual description skipped method: https://docs.ruby-lang.org/en/master/Complex.html#method-i-angle

In other words, minor aliases that is not so worth documented, is skipped.
On the other hand, token_stream seems to be linked to the most major method.
If the most major method is skipped == treated as most minor, something should be wrong.

<%- end %>

<%- unless method.skip_description? then %>
<div class="method-description">
<%- if method.token_stream then %>
<div class="method-source-code" id="<%= method.html_name %>-source">
<pre class="<%= method.source_language %>" data-language="<%= method.source_language %>"><%= method.markup_code %></pre>
</div>
<%- end %>
<%- if method.mixin_from then %>
<div class="mixin-from">
<%= method.singleton ? "Extended" : "Included" %> from <a href="<%= klass.aref_to(method.mixin_from.path) %>"><%= method.mixin_from.full_name %></a>
Expand Down
8 changes: 3 additions & 5 deletions lib/rdoc/generator/template/darkfish/class.rhtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,15 +163,13 @@
<summary>Source</summary>
</details>
</div>
<div class="method-source-code" id="<%= method.html_name %>-source">
<pre><%= method.markup_code %></pre>
</div>
<%- end %>

<%- unless method.skip_description? then %>
<div class="method-description">
<%- if method.token_stream then %>
<div class="method-source-code" id="<%= method.html_name %>-source">
<pre><%= method.markup_code %></pre>
</div>
<%- end %>
<%- if method.mixin_from then %>
<div class="mixin-from">
<%= method.singleton ? "Extended" : "Included" %> from <a href="<%= klass.aref_to(method.mixin_from.path) %>"><%= method.mixin_from.full_name %></a>
Expand Down