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

Allow the user to set collections_dir to put all collections under one subdirectory #6331

Merged
merged 3 commits into from Sep 24, 2017

Conversation

Projects
None yet
8 participants
@parkr
Copy link
Member

parkr commented Aug 23, 2017

Replaces #3723 for Jekyll v3.6 if we want.

User configures as:

collections_dir: my_collections

Then we look in my_collections/_pizza for the pizza collection, and my_collections/_lasagna for the lasagna collection.

/cc @jekyll/build

@@ -100,15 +100,15 @@ def filtered_entries
# Returns a String containing the directory name where the collection
# is stored on the filesystem.
def relative_directory
@relative_directory ||= "_#{label}"
@relative_directory ||= Pathname.new(directory).relative_path_from(Pathname.new(site.source)).to_s

This comment has been minimized.

@ashmaroli

ashmaroli Aug 23, 2017

Member

is this a breaking change?

This comment has been minimized.

@mattr-

mattr- Aug 23, 2017

Member

This isn't a breaking change because the functionality is now in the directory method below.

This comment has been minimized.

@parkr

parkr Aug 24, 2017

Member

It'd be a breaking change if plugins did instance_variable_set(:@relative_directory, "blah") and hoped it would affect directory. It reverses their role for sure, but instance variable manipulation like that is 💯 % not supported.

@@ -9,6 +9,7 @@ class Configuration < Hash
# Where things are
"source" => Dir.pwd,
"destination" => File.join(Dir.pwd, "_site"),
"collections_dir" => "",

This comment has been minimized.

@ashmaroli

ashmaroli Aug 23, 2017

Member

in docs/_docs/configuration.md collections_dir has a value of . (interpreted as current location) but its empty here.

This comment has been minimized.

@mattr-

mattr- Aug 23, 2017

Member

an empty string here is fine. If site.config["collections_dir"] is empty, then File.join will do the right things with the paths.

@mattr-

mattr- approved these changes Aug 23, 2017

Copy link
Member

mattr- left a comment

LGTM :shipit:

@mattr-

This comment has been minimized.

Copy link
Member

mattr- commented Aug 23, 2017

You do have a couple of long line violations from Rubocop but I'm willing to ignore those. ¯\_(ツ)_/¯

@DirtyF

This comment has been minimized.

Copy link
Member

DirtyF commented Aug 24, 2017

@mattr- shouldn't we add here # rubocop:disable LineLengththen?

@mattr-

This comment has been minimized.

Copy link
Member

mattr- commented Aug 24, 2017

@DirtyF That would work.

@jekyll jekyll deleted a comment Aug 24, 2017

@Crunch09
Copy link
Member

Crunch09 left a comment

👍 LGTM, nice to see such a small diff is needed for this feature.
I think a test would be good though.

includes_dir: _includes
source: .
destination: ./_site
collections_dir: .

This comment has been minimized.

@ashmaroli

ashmaroli Aug 25, 2017

Member

this should be "" instead of a .

This comment has been minimized.

@pathawks

pathawks Sep 23, 2017

Member

@ashmaroli Isn't it the current directory? (ie: same as source?)

This comment has been minimized.

@ashmaroli

ashmaroli Sep 24, 2017

Member

I was pointing at a discrepancy in lib/jekyll/configuration.rb above and the documentation here..

In lib/jekyll/configuration.rb above, its {"source" => Dir.pwd, "collections_dir" => ""} but here its source: . and collections_dir: .

@DirtyF DirtyF referenced this pull request Sep 1, 2017

Closed

Allow collection folders within category folders #6030

1 of 1 task complete

@jekyll jekyll deleted a comment from ccamacho Sep 15, 2017

@pathawks pathawks added this to the v3.7.0 milestone Sep 23, 2017

DirtyF added some commits Sep 24, 2017

@DirtyF

DirtyF approved these changes Sep 24, 2017

Copy link
Member

DirtyF left a comment

Pure and simple! 😍

@DirtyF

This comment has been minimized.

Copy link
Member

DirtyF commented Sep 24, 2017

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit 0331fb4 into master Sep 24, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jekyllbot jekyllbot deleted the separate-collections-dir branch Sep 24, 2017

jekyllbot added a commit that referenced this pull request Sep 24, 2017

@hosnas

This comment has been minimized.

Copy link

hosnas commented Oct 14, 2017

Can we have multiple directories for collections?

Something like this:

collections_dir: ['coldir/dir1' , 'coldir/dir2']

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment