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

add glob support to include,exclude option #743

Merged
merged 8 commits into from Jan 13, 2013

Conversation

Projects
None yet
7 participants
@mccxj
Contributor

mccxj commented Jan 9, 2013

use File.fnmatch to glob match

File.symlink?(e)
end
end
end
def glob_include?(exps, e)
!(exps.index { |exp| File.fnmatch?(exp, e) }).nil?

This comment has been minimized.

@ixti

ixti Jan 9, 2013

Member

It's better to use Enumerable#any? here:

exps.any? { |exp| File.fnmatch?(exp, e) }
@mccxj

This comment has been minimized.

Contributor

mccxj commented Jan 10, 2013

@ixti great idea! change code to use any? instead

File.symlink?(e)
end
end
end
def glob_include?(exps, e)
exps.any? { |exp| File.fnmatch?(exp, e) })

This comment has been minimized.

@ixti

ixti Jan 10, 2013

Member

You'll have SyntaxError here. Remove closing bracket: }) -> }

@mccxj

This comment has been minimized.

Contributor

mccxj commented Jan 10, 2013

sorry,i edit code with iPad. ^_^

@mattr-

This comment has been minimized.

Member

mattr- commented Jan 10, 2013

👍 This looks fine to me. I believe it also fulfills @qrush's desire to use globs instead of regexps.

File.symlink?(e)
end
end
end
def glob_include?(exps, e)

This comment has been minimized.

@parkr

parkr Jan 10, 2013

Member

I feel like this should be a core extension instead of randomly in Jekyll. Jekyll#glob_include? just feels weirder to me than Array#glob_include? or something.

This comment has been minimized.

@ixti

ixti Jan 10, 2013

Member

I must admin it was my thought as well.
I think it should be Array#glob_include?.

This comment has been minimized.

@mccxj

mccxj Jan 11, 2013

Contributor

My choose is Enumerable#glob_include?

@parkr

This comment has been minimized.

Member

parkr commented Jan 11, 2013

Please consider making the changes as proposed in the diff. I'd be happy to merge if that change is made.

@ghost ghost assigned parkr Jan 12, 2013

@parkr

This comment has been minimized.

Member

parkr commented Jan 13, 2013

Thanks for doing that. I am working on figuring out which test is failing, but Cucumber is proving fickle.

parkr added a commit that referenced this pull request Jan 13, 2013

Merge pull request #743 from mccxj/master
add glob support to include, exclude option

@parkr parkr merged commit e383bfe into jekyll:master Jan 13, 2013

1 check failed

default The Travis build failed
Details
@AlexanderEkdahl

This comment has been minimized.

Contributor

AlexanderEkdahl commented on 3cf29ee Feb 4, 2013

Previously you could do:

exclude: README.md, Rakefile, Gemfile, Gemfile.lock

But now it has to be an array like this:

exclude:
  - README.md
  - Rakefile
  - Gemfile
  - Gemfile.lock

I like the old ways better and it really used the dynamism of ruby. But I agree that this probably makes more sense code-structure wise.

Thoughts?

This comment has been minimized.

gummesson replied Feb 4, 2013

Just an FYI, but this works too:

exclude: [README.md, Rakefile, Gemfile, Gemfile.lock]

This comment has been minimized.

Contributor

AlexanderEkdahl replied Feb 4, 2013

Thank you. Although I still think this is a regression.

This comment has been minimized.

Member

parkr replied Feb 4, 2013

I mean, we could allow for String support, but the array makes more sense. The purpose of this commit is to add wildcard, i.e. "glob", support so you could do regular-expression-based excludes. It was tedious to have to list each file you wished to exclude.

This comment has been minimized.

Contributor

AlexanderEkdahl replied Feb 4, 2013

Adding String support would be unnecessary bloat. The regular-expression thing is cool and clearly an improvement.

👍

@jhauraw jhauraw referenced this pull request Apr 12, 2013

Closed

How to exclude filetypes? #948

@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.