-
Notifications
You must be signed in to change notification settings - Fork 447
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.
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
Fix method source code not created if a method's description is skipped #1488
Conversation
…description is skipped
…cription is skipped
|
🚀 Preview deployment available at: https://9374ed67.rdoc-6cd.pages.dev (commit: 0837c66) |
st0012
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
| </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> |
There was a problem hiding this comment.
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 %>There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 %> 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Thank you 👍 |
This PR resolves #1335.