Fix --unpublished not affecting collection documents #7027
Conversation
As described in Issue jekyll#7026
|
Thanks for opening the PR. I'd like to leave a few comments regarding the included cucumber test:
|
… unrendered scenarios
|
Thanks for the feedback! I have created a second test to account for the scenario when |
|
The scenarios might look intimidating at first, but it seems just adequate to me. Do not worry about the commit count. Our bot will squash them when merging.. |
|
In case anyone's wondering why the proposed fix doesn't involve the This will be handled by updating #6997 for |
| Then I should get a zero exit status | ||
| And the _site directory should exist | ||
| And I should see "<div>Rover</div>" in "_site/index.html" | ||
| But I should see "<div>Snowy</div>" in "_site/index.html" |
There was a problem hiding this comment.
A document with a future date gets published even without --future option?
There was a problem hiding this comment.
I don't think it gets output, but it does get rendered and is accessible via Liquid. It's kind of weird behavior. 🤷♂️
Why is that? |
@DirtyF For example, one might have a collection labelled "events" with documents for past, current and upcoming events which are then rendered using Liquid Prior discussion: #6506 (comment) |
lib/jekyll/collection.rb
Outdated
| @@ -212,7 +212,9 @@ def container | |||
| def read_document(full_path) | |||
| doc = Document.new(full_path, :site => site, :collection => self) | |||
| doc.read | |||
| docs << doc unless doc.data["published"] == false | |||
| unless doc.data["published"] == false && !site.unpublished | |||
There was a problem hiding this comment.
This unless makes my head hurt.
"Unless not the site is unpublished"
There was a problem hiding this comment.
if site.unpublished || doc.data["published"] != false
docs << doc
end❓
There was a problem hiding this comment.
Reversing the conditional leads us to another cleaner alternative:
if site.unpublished || doc.published?
docs << doc
endThere was a problem hiding this comment.
Agreed, just pushed up a commit that amends this.
|
Wow, great work here! I appreciate the tests that have been added 🎉 |
|
@jekyllbot: |
+fix
Fixes #7026
Note that the changes to
collection.rbdifferent slightly to what was suggested — thefmtlinter gave me a warning against using parens withunless. Please let me know if any further modifications are needed.As a sidenote thanks for the very quick and friendly prompts for how to add this PR. Getting a chance to learn about Cucumber was great!