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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config include trailing slash #8113

Merged
merged 5 commits into from Apr 27, 2020

Conversation

billykong
Copy link
Contributor

This is a 馃悰 bug fix.

  • I've added tests
  • The test suite passes locally

Summary

include in the configuration file should include directory when trailing slash is present, e.g.

# in _config.yml
include:
  - _pages/

should include the directory _pages/ if exists.

The current behavior in the master is that including _pages will include the directory while _pages/ does not.

This PR check that _pages/ matches a directory named _pages but not file.

Context

Resolves #7951

@DirtyF DirtyF added the fix label Apr 13, 2020
@billykong billykong force-pushed the config-include-trailing-slash branch from 4ba5f97 to bf91700 Compare April 13, 2020 23:54
@billykong billykong marked this pull request as ready for review April 14, 2020 00:17
@billykong
Copy link
Contributor Author

Hi @DirtyF, is there any problem with this pull request that needs to be fixed or something I should do? It is my first time going through the pull request workflow in OSS. Please feel free to let me know if I missed something.

@DirtyF DirtyF requested a review from a team April 25, 2020 16:37
@DirtyF
Copy link
Member

DirtyF commented Apr 25, 2020

@billykong nope, we're good, we just need to take the time to review your submission.

test/test_entry_filter.rb Outdated Show resolved Hide resolved
test/test_entry_filter.rb Outdated Show resolved Hide resolved
@billykong
Copy link
Contributor Author

Thanks @DirtyF and @ashmaroli

@ashmaroli
Copy link
Member

ashmaroli commented Apr 26, 2020

@billykong Can you test separately if the issue you're resolving here can be reproduced with Jekyll 3.8.6..? (Want to see if the issue is a regression in Jekyll 4.0)..
Thanks.

@billykong
Copy link
Contributor Author

can be reproduced with Jekyll 3.8.6..?

Hi @ashmaroli , it can be reproduced in Jekyll 3.8.6

@billykong
Copy link
Contributor Author

Tested with Jekyll 3.8.6.

With Trailing slash directory:
jekyll-3 8 6-with-trailing-slash-marked
The _pages directory is not generated.

Without Trailing slash directory:
jekyll-3 8 6-without-trailing-slash-marked
The _pages directory is generated.

@ashmaroli
Copy link
Member

Thanks for testing with Jekyll 3.8.6 @billykong, So it is certain now that the bug existed prior to Jekyll 4.0 and Jekyll 3.8.6 馃憤

Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

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

Thanks @billykong

@DirtyF DirtyF added this to the 4.1 milestone Apr 26, 2020
@ashmaroli
Copy link
Member

Thanks @billykong
@jekyllbot: merge +bug

@jekyllbot jekyllbot merged commit 7087032 into jekyll:master Apr 27, 2020
@jekyllbot jekyllbot added the bug label Apr 27, 2020
jekyllbot added a commit that referenced this pull request Apr 27, 2020
@jekyll jekyll locked and limited conversation to collaborators Apr 27, 2021
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.

Include configuration syntax for hidden folders
4 participants