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

Enable cross reference in code #1240

Merged
merged 1 commit into from
Dec 17, 2024
Merged

Enable cross reference in code #1240

merged 1 commit into from
Dec 17, 2024

Conversation

nobu
Copy link
Member

@nobu nobu commented Dec 14, 2024

Some people like to mark up method names in MarkDown style block quotes, like this: ruby/ruby#12333.
Currently, no links are created in the code in the RDoc, but such words most likely refer to methods.
This PR makes a word a code cross-reference if the whole word can be resolved as a reference.

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.

I have a few questions/concerns:

  • Given the place this changes are made, I think it'd be applied to markdown's too?
  • Will these additional linking attempts cause significant slowdown when generating docs for large projects, like ruby/ruby
  • We need to make sure when a code block doesn't link, it's still rendered as code block.
    • Currently, it breaks most of the inline code blocks in MarkupReference.html
      Screenshot 2024-12-14 at 12 06 25

      They should be:

      Screenshot 2024-12-14 at 12 09 17

case item
when RDoc::Markup::AttrChanger then
# Make "+Class#method+" a cross reference
if respond_to?(:cross_reference) and
Copy link
Member

Choose a reason for hiding this comment

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

Will this ever be false?

@nobu
Copy link
Member Author

nobu commented Dec 15, 2024

  • Given the place this changes are made, I think it'd be applied to markdown's too?

Yes, as mentioned in the description, the motivation is to mark up in markdown, but not only for it.

  • Will these additional linking attempts cause significant slowdown when generating docs for large projects, like ruby/ruby

There was no significant difference, 27sec -> 27sec on my machine.
Probably, already handle_regexp_CROSSREF does similar linking attempts, and marking up as code suppresses it.

  • We need to make sure when a code block doesn't link, it's still rendered as code block.

Fixed.

@nobu nobu requested a review from st0012 December 15, 2024 07:47
Some people like to mark up method names in MarkDown style block
quotes, like this: ruby/ruby#12333.
Currently, no links are created in the code in the RDoc, but such
words most likely refer to methods.
This PR makes a word a code cross-reference if the whole word can be
resolved as a reference.
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.

Thanks for the feature!
In ruby/ruby, it doesn't affect existing documentation much as far as I can tell. But now it links to many methods in news 👍

master

Screenshot 2024-12-17 at 21 45 32

This branch

Screenshot 2024-12-17 at 21 45 35

@st0012 st0012 merged commit 7d7efb0 into ruby:master Dec 17, 2024
22 checks passed
matzbot pushed a commit to ruby/ruby that referenced this pull request Dec 17, 2024
(ruby/rdoc#1240)

Some people like to mark up method names in MarkDown style block
quotes, like this: #12333.
Currently, no links are created in the code in the RDoc, but such
words most likely refer to methods.
This PR makes a word a code cross-reference if the whole word can be
resolved as a reference.

ruby/rdoc@7d7efb0709
@nobu nobu deleted the code-crossref branch December 18, 2024 00:08
nobu added a commit to nobu/rdoc that referenced this pull request Dec 31, 2024
st0012 pushed a commit that referenced this pull request Dec 31, 2024
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.

2 participants