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

Add autolink_excluded_words option to ignore cross-references #1259

Merged
merged 1 commit into from
Dec 31, 2024

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Dec 28, 2024

This config will be handy when the project name is the same as a class or module name, which is often the case for most of the projects.

Closes #1254

Comment on lines 43 to 44
result = @to.convert '+C1+'
assert_equal para("<code>C1</code>"), result
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better that this is linked?
Simple word like "Ruby", "Set" and "Rails" may not be classes/modules, but code like +Ruby+, +Set+ and +Rails+ (or `Ruby`, `Set` and `Rails` in Markdown format) are very likely classes/modules, I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

While I understand the argument, I think it makes the behaviour harder to understand for users. If both C1 and +C1+ are autolinked, I'd expect the exclusion to happen to both of them as well.

Also, some people, including me, often use markdown's code to highlight & emphasize certain keywords without expecting linking. And that causes examples like this:

Screenshot 2024-12-29 at 11 48 19

So I'd expect this config to fix that behavior too.

Copy link
Member

Choose a reason for hiding this comment

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

While I understand the argument, I think it makes the behaviour harder to understand for users. If both C1 and +C1+ are autolinked, I'd expect the exclusion to happen to both of them as well.

These are different.
C1 does not need to be the whole code, it is auto-linked in a sentence the C1 class.
However +C1+ must be the whole code, +the C1 class+ is not linked.

I mean to limit the config to plain texts.

Also, some people, including me, often use markdown's code to highlight & emphasize certain keywords without expecting linking. And that causes examples like this:
Screenshot 2024-12-29 at 11 48 19

The word "Ruby" is not marked as code in that example, is it?
These "Ruby" will not be linked in my proposal.

(and I think we should discourage use of markdown's code just to highlight & emphasize non-code)

So I'd expect this config to fix that behavior too.

I'm not against the config itself.
But a new directive at that module feels better to me.

Copy link
Member

Choose a reason for hiding this comment

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

diff --git i/lib/rdoc/markup/to_html_crossref.rb w/lib/rdoc/markup/to_html_crossref.rb
index 76714522..678e5ee1 100644
--- i/lib/rdoc/markup/to_html_crossref.rb
+++ w/lib/rdoc/markup/to_html_crossref.rb
@@ -92,6 +92,8 @@ class RDoc::Markup::ToHtmlCrossref < RDoc::Markup::ToHtml
       return name if name =~ /\A[a-z]*\z/
     end
 
+    return name if @options.autolink_excluded_words&.include?(name)
+
     cross_reference name, rdoc_ref: false
   end
 
@@ -151,8 +153,6 @@ class RDoc::Markup::ToHtmlCrossref < RDoc::Markup::ToHtml
       label = $'
     end
 
-    return name if name == text && @options.autolink_excluded_words&.include?(name)
-
     ref = @cross_reference.resolve name, text if name
 
     case ref
diff --git i/test/rdoc/test_rdoc_markup_to_html_crossref.rb w/test/rdoc/test_rdoc_markup_to_html_crossref.rb
index cf95ebf3..e50002e8 100644
--- i/test/rdoc/test_rdoc_markup_to_html_crossref.rb
+++ w/test/rdoc/test_rdoc_markup_to_html_crossref.rb
@@ -41,7 +41,7 @@ class RDocMarkupToHtmlCrossrefTest < XrefTestCase
     assert_equal para("C1"), result
 
     result = @to.convert '+C1+'
-    assert_equal para("<code>C1</code>"), result
+    assert_equal para("<a href=\"C1.html\"><code>C1</code></a>"), result
 
     # Explicit linking with rdoc-ref is not ignored
     result = @to.convert 'Constant[rdoc-ref:C1]'

Copy link
Member Author

Choose a reason for hiding this comment

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

So in the case of +C1+, it's an explicit cross-ref linking syntax now. Can you update this document to mention it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated the implementation to follow this expectation.

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

This config will be handy when the project name is the same as a class or
module name, which is often the case for most of the projects.
@nobu
Copy link
Member

nobu commented Dec 31, 2024

Another idea: #1264

@st0012 st0012 mentioned this pull request Dec 31, 2024
@st0012 st0012 merged commit ce77f51 into master Dec 31, 2024
47 checks passed
@st0012 st0012 deleted the fix-#1254 branch December 31, 2024 12:17
matzbot pushed a commit to ruby/ruby that referenced this pull request Dec 31, 2024
cross-references
(ruby/rdoc#1259)

This config will be handy when the project name is the same as a class or
module name, which is often the case for most of the projects.

ruby/rdoc@ce77f51f63
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.

Allow projects to disable auto-linking on certain words
3 participants