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
Refactor collections_dir feature for consistency #6685
Changes from 1 commit
a09b646
39d0bad
2af6834
f4daf2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,157 @@ | ||
Feature: Collections Directory | ||
As a hacker who likes to structure content without clutter | ||
I want to be able to organize my collections under a single directory | ||
And render them from there | ||
|
||
Scenario: Custom collections_dir containing only posts | ||
And I have a collections/_posts directory | ||
And I have the following post within the "collections" directory: | ||
| title | date | content | | ||
| Gathered Post | 2009-03-27 | Random Content. | | ||
And I have a "_config.yml" file with content: | ||
""" | ||
collections_dir: collections | ||
""" | ||
When I run jekyll build | ||
Then I should get a zero exit status | ||
And the _site directory should exist | ||
And I should see "Random Content." in "_site/2009/03/27/gathered-post.html" | ||
|
||
Scenario: Rendered collection in custom collections_dir also containing posts | ||
Given I have a collections/_puppies directory | ||
And I have the following document under the "puppies" collection within the "collections" directory: | ||
| title | date | content | | ||
| Rover | 2007-12-31 | content for Rover. | | ||
And I have a collections/_posts directory | ||
And I have the following post within the "collections" directory: | ||
| title | date | content | | ||
| Gathered Post | 2009-03-27 | Random Content. | | ||
And I have a "_config.yml" file with content: | ||
""" | ||
collections: | ||
puppies: | ||
output: true | ||
|
||
collections_dir: collections | ||
""" | ||
When I run jekyll build | ||
Then I should get a zero exit status | ||
And the _site directory should exist | ||
And the "_site/puppies/rover.html" file should exist | ||
And I should see "Random Content." in "_site/2009/03/27/gathered-post.html" | ||
|
||
Scenario: Rendered collection in custom collections_dir with posts at the site root | ||
Given I have a collections/_puppies directory | ||
And I have the following document under the "puppies" collection within the "collections" directory: | ||
| title | date | content | | ||
| Rover | 2007-12-31 | content for Rover. | | ||
And I have a _posts directory | ||
And I have the following post: | ||
| title | date | content | | ||
| Post At Root | 2009-03-27 | Random Content. | | ||
And I have a "_config.yml" file with content: | ||
""" | ||
collections: | ||
puppies: | ||
output: true | ||
|
||
collections_dir: collections | ||
""" | ||
When I run jekyll build | ||
Then I should get a zero exit status | ||
And the _site directory should exist | ||
And the "_site/puppies/rover.html" file should exist | ||
And the "_site/post-at-root.html" file should not exist | ||
And the _site/_posts directory should not exist | ||
|
||
Scenario: Rendered collection in custom collections_dir also containing drafts | ||
Given I have a collections/_puppies directory | ||
And I have the following document under the "puppies" collection within the "collections" directory: | ||
| title | date | content | | ||
| Rover | 2007-12-31 | content for Rover. | | ||
And I have a collections/_drafts directory | ||
And I have the following draft within the "collections" directory: | ||
| title | date | content | | ||
| Gathered Draft | 2009-03-27 | Random Content. | | ||
And I have a "_config.yml" file with content: | ||
""" | ||
collections: | ||
puppies: | ||
output: true | ||
|
||
collections_dir: collections | ||
""" | ||
When I run jekyll build --drafts | ||
Then I should get a zero exit status | ||
And the _site directory should exist | ||
And the "_site/puppies/rover.html" file should exist | ||
And I should see "Random Content." in "_site/2009/03/27/gathered-draft.html" | ||
And the _site/collections directory should not exist | ||
|
||
Scenario: Rendered collection in custom collections_dir with drafts at the site root | ||
Given I have a collections/_puppies directory | ||
And I have the following document under the "puppies" collection within the "collections" directory: | ||
| title | date | content | | ||
| Rover | 2007-12-31 | content for Rover. | | ||
And I have a _drafts directory | ||
And I have the following draft: | ||
| title | date | content | | ||
| Draft At Root | 2009-03-27 | Random Content. | | ||
And I have a "_config.yml" file with content: | ||
""" | ||
collections: | ||
puppies: | ||
output: true | ||
|
||
collections_dir: collections | ||
""" | ||
When I run jekyll build --drafts | ||
Then I should get a zero exit status | ||
And the _site directory should exist | ||
And the "_site/puppies/rover.html" file should exist | ||
And the "_site/2009/03/27/draft-at-root.html" file should not exist | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
Scenario: A complex site with collections posts and drafts at various locations | ||
Given I have a gathering/_puppies directory | ||
And I have a gathering/_posts directory | ||
And I have a gathering/_drafts directory | ||
And I have a _puppies directory | ||
And I have a _posts directory | ||
And I have a _drafts directory | ||
And I have the following document under the "puppies" collection within the "gathering" directory: | ||
| title | date | content | | ||
| Rover in Gathering | 2007-12-31 | content for Rover. | | ||
And I have the following document under the puppies collection: | ||
| title | date | content | | ||
| Rover At Root | 2007-12-31 | content for Rover. | | ||
And I have the following post within the "gathering" directory: | ||
| title | date | content | | ||
| Post in Gathering | 2009-03-27 | Totally nothing. | | ||
And I have the following post: | ||
| title | date | content | | ||
| Post At Root | 2009-03-27 | Totally nothing. | | ||
And I have the following draft within the "gathering" directory: | ||
| title | date | content | | ||
| Draft In Gathering | 2009-03-27 | This is a draft. | | ||
And I have the following draft: | ||
| title | date | content | | ||
| Draft At Root | 2009-03-27 | This is a draft. | | ||
And I have a "_config.yml" file with content: | ||
""" | ||
collections: | ||
puppies: | ||
output: true | ||
|
||
collections_dir: gathering | ||
""" | ||
When I run jekyll build --drafts | ||
Then I should get a zero exit status | ||
And the _site directory should exist | ||
And the "_site/puppies/rover-in-gathering.html" file should exist | ||
And the "_site/2009/03/27/post-in-gathering.html" file should exist | ||
And the "_site/2009/03/27/draft-in-gathering.html" file should exist | ||
And the "_site/gathering/2009/03/27/draft-in-gathering.html" file should not exist | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A couple of lines below we check that the |
||
And the "_site/2009/03/27/draft-at-root.html" file should not exist | ||
But the "_site/puppies/rover-at-root.html" file should not exist | ||
And the _site/gathering directory should not exist | ||
And the _site/_posts directory should not exist |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,6 +95,20 @@ | |
|
||
# | ||
|
||
Given(%r!^I have the following (draft|post)s? within the "(.*)" directory:$!) do |type, folder, table| | ||
table.hashes.each do |input_hash| | ||
title = slug(input_hash["title"]) | ||
parsed_date = Time.xmlschema(input_hash["date"]) rescue Time.parse(input_hash["date"]) | ||
|
||
filename = type == "draft" ? "#{title}.markdown" : "#{parsed_date.strftime("%Y-%m-%d")}-#{title}.markdown" | ||
|
||
path = File.join(folder, "_#{type}s", filename) | ||
File.write(path, file_content_from_hash(input_hash)) | ||
end | ||
end | ||
|
||
# | ||
|
||
Given(%r!^I have the following documents? under the (.*) collection:$!) do |folder, table| | ||
table.hashes.each do |input_hash| | ||
title = slug(input_hash["title"]) | ||
|
@@ -108,6 +122,16 @@ | |
|
||
# | ||
|
||
Given(%r!^I have the following documents? under the "(.*)" collection within the "(.*)" directory:$!) do |label, dir, table| | ||
table.hashes.each do |input_hash| | ||
title = slug(input_hash["title"]) | ||
path = File.join(dir, "_#{label}", "#{title}.md") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason why you chose There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. only because that's how our default site gets created from |
||
File.write(path, file_content_from_hash(input_hash)) | ||
end | ||
end | ||
|
||
# | ||
|
||
Given(%r!^I have a configuration file with "(.*)" set to "(.*)"$!) do |key, value| | ||
config = \ | ||
if source_dir.join("_config.yml").exist? | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -95,14 +95,13 @@ def filtered_entries | |||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# The directory for this Collection, relative to the site source. | ||||||||||||||||||||||||||||
# The directory for this Collection, relative to the site source or the directory | ||||||||||||||||||||||||||||
# containing the collection. | ||||||||||||||||||||||||||||
# | ||||||||||||||||||||||||||||
# Returns a String containing the directory name where the collection | ||||||||||||||||||||||||||||
# is stored on the filesystem. | ||||||||||||||||||||||||||||
def relative_directory | ||||||||||||||||||||||||||||
@relative_directory ||= Pathname.new(directory).relative_path_from( | ||||||||||||||||||||||||||||
Pathname.new(site.source) | ||||||||||||||||||||||||||||
).to_s | ||||||||||||||||||||||||||||
@relative_directory ||= "_#{label}" | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be relative to the site source, so if source is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Things that're going to break if Lines 32 to 36 in f77d704
Lines 76 to 79 in f77d704
Lines 121 to 124 in f77d704
|
||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# The full path to the directory containing the collection. | ||||||||||||||||||||||||||||
|
@@ -111,7 +110,7 @@ def relative_directory | |||||||||||||||||||||||||||
# is stored on the filesystem. | ||||||||||||||||||||||||||||
def directory | ||||||||||||||||||||||||||||
@directory ||= site.in_source_dir( | ||||||||||||||||||||||||||||
File.join(site.config["collections_dir"], "_#{label}") | ||||||||||||||||||||||||||||
File.join(site.config["collections_dir"], relative_directory) | ||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
|
@@ -125,7 +124,7 @@ def directory | |||||||||||||||||||||||||||
# is stored on the filesystem. | ||||||||||||||||||||||||||||
def collection_dir(*files) | ||||||||||||||||||||||||||||
return directory if files.empty? | ||||||||||||||||||||||||||||
site.in_source_dir(relative_directory, *files) | ||||||||||||||||||||||||||||
site.in_source_dir(site.config["collections_dir"], relative_directory, *files) | ||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Checks whether the directory "exists" for this collection. | ||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -83,7 +83,7 @@ def draft? | |
# Returns a String path which represents the relative path | ||
# from the site source to this document | ||
def relative_path | ||
@relative_path ||= Pathutil.new(path).relative_path_from(site.source).to_s | ||
@relative_path ||= Pathutil.new(path).relative_path_from(focal_point).to_s | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why can't this be the site.source? Are you thinking the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason it can't have that method? I don't like the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I did not want to introduce a new There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well now that we have introduce a potential breaking change in a minor version, we should be above that kind of consideration here. The more There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @parkr Your call.. should I introduce a new public_method in a patch-release..? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think adding Collection#path to fix this bug is a fine solution for a patch release. SemVer makes no assumptions about methods being added, as far as I can tell. Our SemVer is guided by new features (minor bump) and breaking changes (major bump). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would call "enhancing" That said, this is not the place for debating SemVer.. your comment above is clear enough an answer for how to proceed.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small change of plans.. Instead of having |
||
end | ||
|
||
# The output extension of the document. | ||
|
@@ -399,11 +399,9 @@ def categories_from_path(special_dir) | |
def populate_categories | ||
merge_data!({ | ||
"categories" => ( | ||
Array(data["categories"]) + Utils.pluralized_array_from_hash( | ||
data, | ||
"category", | ||
"categories" | ||
) | ||
Array(data["categories"]) + Utils.pluralized_array_from_hash( | ||
data, "category", "categories" | ||
) | ||
).map(&:to_s).flatten.uniq, | ||
}) | ||
end | ||
|
@@ -414,6 +412,13 @@ def populate_tags | |
}) | ||
end | ||
|
||
private | ||
def focal_point | ||
collections_dir = site.config["collections_dir"] | ||
@focal_point ||= | ||
collections_dir.empty? ? site.source : site.in_source_dir(collections_dir) | ||
end | ||
|
||
private | ||
def merge_categories!(other) | ||
if other.key?("categories") && !other["categories"].nil? | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,6 +62,7 @@ def read_directories(dir = "") | |
# | ||
# Returns nothing. | ||
def retrieve_posts(dir) | ||
return if not_within_configured_directory?(dir) | ||
site.posts.docs.concat(PostReader.new(site).read_posts(dir)) | ||
site.posts.docs.concat(PostReader.new(site).read_drafts(dir)) if site.show_drafts | ||
end | ||
|
@@ -130,5 +131,20 @@ def get_entries(dir, subfolder) | |
entries = Dir.chdir(base) { filter_entries(Dir["**/*"], base) } | ||
entries.delete_if { |e| File.directory?(site.in_source_dir(base, e)) } | ||
end | ||
|
||
private | ||
|
||
# Internal | ||
# | ||
# Determine if the directory is supposed to contain posts and drafts. | ||
# If the user has defined a custom collections_dir, then attempt to read | ||
# posts and drafts only from within that directory. | ||
# | ||
# Returns true if a custom collections_dir has been set but current directory lies | ||
# outside that directory. | ||
def not_within_configured_directory?(dir) | ||
collections_dir = site.config["collections_dir"] | ||
!collections_dir.empty? && !dir.start_with?("/#{collections_dir}") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think of renaming this method to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it can be done.. I kept it negative so that human can readily parse two negative expressions easily in comparison to one negative and the other positive.. def within_configured_directory?(dir)
collections_dir = site.config["collections_dir"]
!collections_dir.empty? && dir.start_with?("/#{collections_dir}")
end |
||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand it doesn't exist anyway but shouldn't that be
_site/2009/03/27/post-at-root.html
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes.. it should.. we ought to be checking for the expected outcomes.. 👍