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

Excludes should be relative to the site source #1916

Merged
merged 5 commits into from
Jan 11, 2014
Merged

Conversation

parkr
Copy link
Member

@parkr parkr commented Jan 5, 2014

Fixes #1915.

Should we do the same for include entries?

@ghost ghost assigned mattr- Jan 5, 2014
@parkr
Copy link
Member Author

parkr commented Jan 5, 2014

Thoughts on removing glob_include? from core extensions and moving it to EntryFilter, @mattr-?

@mattr-
Copy link
Member

mattr- commented Jan 8, 2014

Seems fine to move it to EntryFilter.

I'm also thinking that we should do the same thing for include entries as well (make them relative to the site source).

@ghost ghost assigned parkr Jan 8, 2014
@mattr-
Copy link
Member

mattr- commented Jan 8, 2014

Did you want to add include support before this gets merged?

@parkr
Copy link
Member Author

parkr commented Jan 9, 2014

@mattr- No, we'll take care of includes in another PR.

@parkr
Copy link
Member Author

parkr commented Jan 9, 2014

/cc @benbalter

@mattr- mattr- merged commit d96165e into master Jan 11, 2014
mattr- added a commit that referenced this pull request Jan 11, 2014
@parkr parkr deleted the gitignore-excludes branch January 11, 2014 19:31
@benbalter
Copy link
Contributor

May want to update the docs with the new behavior?

@mattr-
Copy link
Member

mattr- commented Jan 11, 2014

Yes! Good call!

@mattr-
Copy link
Member

mattr- commented Jan 11, 2014

Updated the docs in f3e9eb9

Here's what the new section in the config documentation looks like:
screen shot 2014-01-11 at 3 04 43 pm

@parkr
Copy link
Member Author

parkr commented Jan 11, 2014

Thanks! ❤️

@jbranchaud
Copy link
Contributor

Just to make sure I am clear on how this works. Let's say I have a jekyll project with css/, assets/css/ and the rest of the standard jekyll stuff and my _config.yml has an exclude like the following:

exclude: ['css']

In the current version of jekyll, this exclude would exclude both css/ and assets/css/, despite only intending to exclude the former. Does the new behavior addressed by this issue mean that only the first will be excluded in my scenario? Also, does one need to use something like ./ to notate a relative directory path or is this implicit?

@parkr
Copy link
Member Author

parkr commented Feb 14, 2014

Want to add a test for your use case in a new PR? :)

@jbranchaud
Copy link
Contributor

I could probably do that, but I wanted to make sure I understood the intent of this issue/PR. What did you have in mind when you were proposing and adding this behavior?

@parkr
Copy link
Member Author

parkr commented Feb 14, 2014

@jbranchaud
Copy link
Contributor

@parkr I have added some test cases in #2046.

alexmuller referenced this pull request in alphagov/government-service-design-manual Jul 28, 2014
Excluding all directories named vendor was excluding the vendor
directory in the toolkit, which is where the jQuery that plays
videos lives.
@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot specify root directory within exclude directive
5 participants