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

Fix mangling of whitespace when site.lang is set #110

Merged
merged 3 commits into from
Apr 18, 2016
Merged

Conversation

benbalter
Copy link
Contributor

Which slams hreflang into the preceding attribute, causing parse errors.

This just adds a quick negative look behind to change the whitespace stripping behavior.

@@ -32,7 +32,7 @@ def source_path

def feed_content
feed = PageWithoutAFile.new(@site, File.dirname(__FILE__), "", path)
feed.content = File.read(source_path).gsub(/\s+([{<])/, '\1')
feed.content = File.read(source_path).gsub(/(?<!\")\s+([<{])/, '\1')
Copy link
Member

Choose a reason for hiding this comment

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

Would it have also worked to put the space on the other side of the hreflang, i.e. at the beginning (right after {% if %}) instead of at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no way we can remember to always do that going forward, nor could we expect contributors to deal with an odd behavior like that.

Copy link
Member

Choose a reason for hiding this comment

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

It's not ideal, true. 👍

@parkr
Copy link
Member

parkr commented Apr 18, 2016

YOU'RE A HERO thank you

@jekyllbot: merge

@jekyllbot jekyllbot merged commit a6c1968 into master Apr 18, 2016
@jekyllbot jekyllbot deleted the whitespace-fix branch April 18, 2016 22:03
jekyllbot added a commit that referenced this pull request Apr 18, 2016
@jekyll jekyll locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants