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.

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

Jump to bottom

Fix/i18n headers #67

Merged
merged 8 commits into from Jul 22, 2017
Merged

Fix/i18n headers #67

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4429d46
alpha changes
untra Jul 18, 2017
be6b649
gitignore
untra Jul 18, 2017
2f5067d
removed site/_site files
untra Jul 18, 2017
2016efe
initial work on relative urls
untra Jul 19, 2017
2ca3c91
split absolute url parsing from relative url parsing
untra Jul 22, 2017
b033972
added documentation
untra Jul 22, 2017
dceb57d
cleaned up readme
untra Jul 22, 2017
2a26a02
fixed missing baseurl from absolute regex
untra Jul 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .rubocop.yml
View file
Open in desktop
Expand Up @@ -69,7 +69,7 @@ Metrics/LineLength:
- rake/*.rake
- Gemfile
- jekyll.gemspec
Max: 90
Max: 100
Severity: warning
Metrics/MethodLength:
CountComments: false
Expand Down
8 changes: 7 additions & 1 deletion README.md
View file
Open in desktop
Expand Up @@ -90,7 +90,13 @@ becomes
Notice the link `<a href="/fr/menu/">...` directs to the french website.

Even if you are falling back to `default_lang` page, relative links built on the *french* site will
still link to *french* pages.
still link to *french* pages.v
Copy link
Collaborator

Choose a reason for hiding this comment

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

The last v looks odd.


#### Relativized Absolute Urls
If you defined a site `url` in your `_config.yaml`, polyglot will automatically relativize links pointing to your absolute site url. If you don't want them relativized, adding a space explicitly to an href prevents the the absolute url from being relativized.

processed: `href="http://mywebsite.com/about"`
unprocessed: `href=" http://mywebsite.com/about"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should the space trick work for absolute URLs only? Frankly speaking, I think it is easier to understand in a form of "in case you want to keep the URL as is, just prepend it with a whitespace and polyglot would ignore the link".

If whitespace trick works for all kinds of URLs, then the wording should probably be improved. Currently it sounds like the feature is absolute-URL-only.


#### Localized site.data

Expand Down
12 changes: 6 additions & 6 deletions lib/jekyll/polyglot/liquid/tags/i18n-headers.rb
View file
Open in desktop
Expand Up @@ -12,14 +12,14 @@ def initialize(tag_name, text, tokens)
def render(context)
site = context.registers[:site]
permalink = context.registers[:page]['permalink']
siteUrl = (@url.empty?) ? site.config['url'] : @url
i18n = "<meta http-equiv=\"Content-Language\" content=\"#{site.active_lang}\">"
i18n += "<link rel=\"alternate\" i18n=\"#{site.default_lang}\""\
" href=\" #{siteUrl}#{permalink}\" />\n"
site_url = @url.empty? ? site.config['url'] : @url
i18n = "<meta http-equiv=\"Content-Language\" content=\"#{site.active_lang}\">\n"
i18n += "<link rel=\"alternate\" i18n=\"#{site.default_lang}\" "\
"href=\" #{site_url}#{permalink}\"/>\n"
site.languages.each do |lang|
next if lang == site.default_lang
i18n += "<link rel=\"alternate\" i18n=\"#{lang}\""\
" href=\" #{siteUrl}/#{lang}#{permalink}\" />\n"
i18n += "<link rel=\"alternate\" i18n=\"#{lang}\" "\
"href=\"#{site_url}/#{lang}#{permalink}\"/>\n"
end
i18n
end
Expand Down
33 changes: 26 additions & 7 deletions lib/jekyll/polyglot/patches/jekyll/site.rb
View file
Open in desktop
Expand Up @@ -100,8 +100,15 @@ def coordinate_documents(docs)

# performs any necesarry operations on the documents before rendering them
def process_documents(docs)
return if @active_lang == @default_lang
url = config.fetch('url', false)
rel_regex = relative_url_regex
abs_regex = absolute_url_regex(url)
docs.each do |doc|
relativize_urls doc
relativize_urls(doc, rel_regex)
if url
then relativize_absolute_urls(doc, abs_regex, url)
end
end
end

Expand All @@ -125,14 +132,26 @@ def relative_url_regex
@exclude.each do |x|
regex += "(?!#{x}\/)"
end
url_quoted = config['url']
url_quoted = Regexp.quote(url_quoted) unless url_quoted.nil?
%r{href=\"(?:#{url_quoted})?#{@baseurl}\/((?:#{regex}[^,'\"\s\/?\.#]+\.?)*(?:\/[^\]\[\)\(\"\'\s]*)?)\"}
%r{href=\"?#{@baseurl}\/((?:#{regex}[^,'\"\s\/?\.#]+\.?)*(?:\/[^\]\[\)\(\"\'\s]*)?)\"}
Copy link
Collaborator

@vlsi vlsi Jul 22, 2017 •
edited

Choose a reason for hiding this comment

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

As far as I can tell, relative URLs like /en/about will still be processed, while absolute URLs that include /:lang are excluded. Is that difference between absolute/relative intentional?
Should this be documented/tested?

Copy link
Owner Author

Choose a reason for hiding this comment

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

It should be intentional. The exclusion of the /:lang from absolute urls is needed to ensure that the i18n_headers are mostly left alone.

You are right thought a relative url like /en/about will be processed into /en/en/about which isn't right. Maybe the relative urls should get the same treatment.

Copy link
Owner Author

Choose a reason for hiding this comment

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

they're getting the same treatment 👍

end

def relativize_urls(doc)
return if @active_lang == @default_lang
doc.output.gsub!(relative_url_regex, "href=\"#{@baseurl}/#{@active_lang}/" + '\1"')
def absolute_url_regex(url)
regex = ''
@exclude.each do |x|
regex += "(?!#{x}\/)"
end
@languages.each do |l|
regex += "(?!#{l}\/)"
end
%r{href=\"?#{url}\/((?:#{regex}[^,'\"\s\/?\.#]+\.?)*(?:\/[^\]\[\)\(\"\'\s]*)?)\"}
Copy link
Owner Author

Choose a reason for hiding this comment

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

should be

%r{href=\"?#{url}#{baseurl}\/((?:#{regex}[^,'\"\s\/?\.#]+\.?)*(?:\/[^\]\[\)\(\"\'\s]*)?)\"}

end

def relativize_urls(doc, regex)
doc.output.gsub!(regex, "href=\"#{@baseurl}/#{@active_lang}/" + '\1"')
end

def relativize_absolute_urls(doc, regex, url)
doc.output.gsub!(regex, "href=\"#{url}/#{@active_lang}/" + '\1"')
Copy link
Owner Author

Choose a reason for hiding this comment

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

should be

doc.output.gsub!(regex, "href=\"#{url}#{baseurl}/#{@active_lang}/" + '\1"')

end
end
end
47 changes: 30 additions & 17 deletions spec/tests/lib/jekyll/polyglot/patches/jekyll/site_spec.rb
View file
Open in desktop
@@ -1,5 +1,6 @@
require "rspec/helper"
require 'rspec/helper'
require 'ostruct'
# rubocop:disable BlockLength, LineLength
describe Site do
before do
@config = Jekyll::Configuration::DEFAULTS.dup
Expand Down Expand Up @@ -117,35 +118,47 @@
expect(@relative_url_regex).to_not match "href=\"#{baseurl}/css/stylesheet.css\""
end
end
end

describe @absolute_url_regex do
it 'must match absolute url' do
@urls.each do |url|
@site.config['url'] = url
@site.prepare
@relative_url_regex = @site.relative_url_regex
expect(@relative_url_regex).to match "href=\"#{url}/javascript/65487-app.js\""
expect(@relative_url_regex).to match "href=\"#{url}/images/my-vacation-photo.jpg\""
expect(@relative_url_regex).to match "href=\"#{url}/css/stylesheet.css\""
@absolute_url_regex = @site.absolute_url_regex(url)
expect(@absolute_url_regex).to match "href=\"#{url}/javascript/65487-app.js\""
expect(@absolute_url_regex).to match "href=\"#{url}/images/my-vacation-photo.jpg\""
expect(@absolute_url_regex).to match "href=\"#{url}/css/stylesheet.css\""
end
end

it 'must match absolute url' do
@urls.each do |url|
@site.config['url'] = url
@relative_url_regex = @site.relative_url_regex
expect(@relative_url_regex).to match "href=\"#{url}/javascript/65487-app.js\""
expect(@relative_url_regex).to match "href=\"#{url}/images/my-vacation-photo.jpg\""
expect(@relative_url_regex).to match "href=\"#{url}/css/stylesheet.css\""
@absolute_url_regex = @site.absolute_url_regex(url)
expect(@absolute_url_regex).to match "href=\"#{url}/javascript/65487-app.js\""
expect(@absolute_url_regex).to match "href=\"#{url}/images/my-vacation-photo.jpg\""
expect(@absolute_url_regex).to match "href=\"#{url}/css/stylesheet.css\""
end
end

it 'must not match absolute url for another project' do
@urls.each do |url|
@site.config['url'] = url
@relative_url_regex = @site.relative_url_regex
expect(@relative_url_regex).to_not match "href=\"http://test_github.io/javascript/65487-app.js\""
expect(@relative_url_regex).to_not match "href=\"http://test_github.io/images/my-vacation-photo.jpg\""
expect(@relative_url_regex).to_not match "href=\"http://github.io/css/stylesheet.css\""
@absolute_url_regex = @site.absolute_url_regex(url)
expect(@absolute_url_regex).to_not match 'href="http://test_github.io/javascript/65487-app.js'
expect(@absolute_url_regex).to_not match 'href="http://test_github.io/images/my-vacation-photo.jpg'
expect(@absolute_url_regex).to_not match 'href="http://github.io/css/stylesheet.css'
end
end

it 'must not match whitespaced urls' do
@urls.each do |url|
@site.config['url'] = url
@absolute_url_regex = @site.absolute_url_regex(url)
expect(@absolute_url_regex).to_not match "href=\" #{url}/javascript/65487-app.js\""
expect(@absolute_url_regex).to_not match "href=\" #{url}/images/my-vacation-photo.jpg\""
expect(@absolute_url_regex).to_not match "href=\" #{url}/css/stylesheet.css\" "
end
end

Expand All @@ -154,10 +167,10 @@
@site.baseurl = baseurl
@urls.each do |url|
@site.config['url'] = url
@relative_url_regex = @site.relative_url_regex
expect(@relative_url_regex).to match "href=\"#{url}#{baseurl}/javascript/65487-app.js\""
expect(@relative_url_regex).to match "href=\"#{url}#{baseurl}/images/my-vacation-photo.jpg\""
expect(@relative_url_regex).to match "href=\"#{url}#{baseurl}/css/stylesheet.css\""
@absolute_url_regex = @site.absolute_url_regex(url)
expect(@absolute_url_regex).to match "href=\"#{url}#{baseurl}/javascript/65487-app.js\""
expect(@absolute_url_regex).to match "href=\"#{url}#{baseurl}/images/my-vacation-photo.jpg\""
expect(@absolute_url_regex).to match "href=\"#{url}#{baseurl}/css/stylesheet.css\""
end
end
end
Expand Down