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

Dest match #1556

merged 3 commits into from Sep 17, 2013


None yet
4 participants

maul-esel commented Sep 16, 2013

As noted in #1130, jekyll's current matching of source and destination (to ensure that the source is not inside the destination) is too aggressive, meaning that a source of blog-source and a destination of blog will cause the generation to halt. This is because it uses a simple regex for checking.

This PR changes the relevant code to use Pathname instead, and also adds an extensive cucumber feature to test the behaviour. This fixes the described issue.

# parent to the source dir.
def ensure_not_in_dest
dest = do |path|

This comment has been minimized.


parkr Sep 16, 2013


How efficient is Pathname#ascend?

This comment has been minimized.


mattr- Sep 17, 2013


as efficient as working with strings, since that's what most of Pathname does anyways.

@@ -137,10 +137,14 @@
Then /^the (.*) directory should exist$/ do |dir|
Then /^the (.*) directory should +exist$/ do |dir|

This comment has been minimized.


This comment has been minimized.


parkr commented Sep 16, 2013

This looks pretty good to me! @mattr-?

Thanks @maul-esel!

mattr- added a commit that referenced this pull request Sep 17, 2013

Merge pull request #1556 from maul-esel/dest-match
Fix up matching against source and destination when the two locations are similar.

@mattr- mattr- merged commit de49342 into jekyll:master Sep 17, 2013

1 check passed

default The Travis CI build passed

mattr- added a commit that referenced this pull request Sep 17, 2013

@maul-esel maul-esel deleted the maul-esel:dest-match branch Sep 17, 2013

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.