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

Layout: set relative_path without using Pathname #5164

Merged
merged 1 commit into from
Aug 2, 2016

Conversation

parkr
Copy link
Member

@parkr parkr commented Jul 29, 2016

Presently, on a Windows machine, you get an ArgumentError on Windows:

     Generating...
C:/Ruby23-x64/lib/ruby/2.3.0/pathname.rb:520:in `relative_path_from':
    different prefix: "/" and "C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/minima-1.0.1" (ArgumentError)
    from C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/jekyll-3.2.0/lib/jekyll/layout.rb:61:in `relative_path'
    from C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/jekyll-3.2.0/lib/jekyll/renderer.rb:161:in `place_in_layouts'
    from C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/jekyll-3.2.0/lib/jekyll/renderer.rb:71:in `run'

This patch attempts to fix that. Creating a PR so I can use AppVeyor.

This doesn't affect filesystems which do not use drive names.

/cc @jekyll/windows

Presently, on a Windows machine, you get an ArgumentError on Windows:

     Generating...
C:/Ruby23-x64/lib/ruby/2.3.0/pathname.rb:520:in `relative_path_from':
    different prefix: "/" and "C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/minima-1.0.1" (ArgumentError)
    from C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/jekyll-3.2.0/lib/jekyll/layout.rb:61:in `relative_path'
    from C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/jekyll-3.2.0/lib/jekyll/renderer.rb:161:in `place_in_layouts'
    from C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/jekyll-3.2.0/lib/jekyll/renderer.rb:71:in `run'

This doesn't affect filesystems which do not use drive names.
@parkr parkr added this to the 3.2.1 milestone Jul 29, 2016
@parkr parkr added the fix label Jul 29, 2016
@parkr
Copy link
Member Author

parkr commented Jul 29, 2016

@XhmikosR Would you mind giving this SHA a try on your site? I believe this should do it. Down to 15 failures on AppVeyor.

Fixes #5146.

@qbikez
Copy link

qbikez commented Jul 30, 2016

This fixed #4677 for me (in a fresh jekyll instance).

@XhmikosR
Copy link
Contributor

@parkr: confirmed, it fixes the issue for me on Windows.

@parkr
Copy link
Member Author

parkr commented Aug 1, 2016

LGTM. @jekyll/core, would you mind giving this another LGTM?

@parkr
Copy link
Member Author

parkr commented Aug 2, 2016

🆒

@jekyllbot: merge +bug

@jekyllbot jekyllbot merged commit 6e0119d into master Aug 2, 2016
@jekyllbot jekyllbot deleted the fix-windows-error branch August 2, 2016 00:34
jekyllbot added a commit that referenced this pull request Aug 2, 2016
@XhmikosR
Copy link
Contributor

XhmikosR commented Aug 2, 2016

@parkr: really appreciate it. Do you think the remaining Windows failures could be fixed so that it's easier to spot any failures in the future?

@parkr
Copy link
Member Author

parkr commented Aug 2, 2016

@XhmikosR Yeah, I would like to do that. I'll put together an issue tomorrow (I'm in SF) and hopefully some of our amazing fellow contributors can lend a hand. 😄

@parkr parkr mentioned this pull request Aug 2, 2016
9 tasks
@jekyll jekyll locked and limited conversation to collaborators Jul 11, 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.

4 participants