-
Notifications
You must be signed in to change notification settings - Fork 440
Add support for canonical URL link tag #1354
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
Conversation
8e8e536
to
2056471
Compare
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.
Love this idea ❤️ Just some suggestions.
<%- if @options.canonical_root -%> | ||
<% context = klass if defined?(klass) %> | ||
<% context = file if defined?(file) %> | ||
<link rel="canonical" href="<%= canonical_url(context) %>"> |
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.
Can this be included in the if statements above? Then we don't need the context conditions separately.
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've changed the implementation to use current
instead of klass
and file
to simplify.
If I'd use the if
statement I would need to add an else
as well, resulting in adding the tag 4 times.
content = File.binread("Klass/Inner.html") | ||
|
||
assert_include(content, '<link rel="canonical" href="https://docs.ruby-lang.org/en/master/Klass/Inner.html">') | ||
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.
Can we have another test for file pages?
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.
Done!
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.
Hmm sorry I don't see it?
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.
Ah sorry, didn't push my changes.
Should be present now.
lib/rdoc/generator/darkfish.rb
Outdated
@@ -729,6 +729,10 @@ def excerpt(comment) | |||
extracted_text[0...150].tr_s("\n", " ").squeeze(" ") | |||
end | |||
|
|||
def canonical_url(context) | |||
File.join(@options.canonical_root, context&.path.to_s) |
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 prefer making canonical_url
taking the path already instead of a nilable context. It'll likely be easier to maintain and debug.
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've changed the implementation to take the path instead.
ab9dded
to
ca5afb8
Compare
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.
+1
84858e4
to
0c82bbe
Compare
I’ve changed the implementation to have less logic in the template and moved it to RDoc::Markup. This will allow other templates to use it as well. |
0c82bbe
to
f07af27
Compare
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.
This is great, thank you!
@p8 can you rebase and push again? I'll merge it then, thx |
Currently, search engines don't know which version to show for API documentation. For example, searching for "ruby string" in Google will show the documentation for Ruby 2.0 `https://docs.ruby-lang.org/en/2.0.0/String.html` as the top result (for docs.ruby-lang.org). The canonical URL link tag will allow us to set the preferred version: > The canonical URL link tag defines the preferred URL for the current > document, which helps search engines reduce duplicate content. https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Attributes/rel#canonical For example, for the official Ruby documentation we can add the following to `.rdoc_options`. ```yaml canonical_root: https://docs.ruby-lang.org/en/master ``` This will add the canonical URL link tag to relevant pages. Co-authored-by: Stan Lo <[email protected]>
f07af27
to
cec51e3
Compare
Thanks @st0012 ! I've rebased to master. |
v6.13.1...master We can then bump `ruby/ruby`'s RDoc to [set canonical url](#1354).
I've applied this to Ruby's master doc and 3.4 doc. |
@st0012 Thanks! |
Currently, search engines don't know which version to show for API documentation. For example, searching for "ruby string" in Google will show the documentation for Ruby 2.0
https://docs.ruby-lang.org/en/2.0.0/String.html
as the top result (for docs.ruby-lang.org).The canonical URL link tag will allow us to set the preferred version:
https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Attributes/rel#canonical
For example, for the official Ruby documentation we can add the following to
.rdoc_options
.This will add the canonical URL link tag to relevant pages: