Merge pull request #2238 from jekyll/fix-the-collections

features/collections.feature

@@ -9,30 +9,64 @@ Feature: Collections
     And I have a configuration file with "collections" set to "['methods']"
     When I run jekyll build
     Then the _site directory should exist
-    And I should see "Collections: Use `{{ page.title }}` to build a full configuration for use w/Jekyll.\n\nWhatever: {{ page.whatever }}\n`{{ page.title }}` is used to make sure your path is in your source.\nRun your generators! {{ page.layout }}\nCreate dat site.\nRun your generators! {{ page.layout }}" in "_site/index.html"
+    And I should see "Collections: <p>Use <code>Jekyll.configuration</code> to build a full configuration for use w/Jekyll.</p>\n\n<p>Whatever: foo.bar</p>\n<p><code>Jekyll.sanitized_path</code> is used to make sure your path is in your source.</p>\n<p>Run your generators! default</p>\n<p>Create dat site.</p>\n<p>Run your generators! default</p>" in "_site/index.html"
+    And the "_site/methods/configuration.html" file should not exist
 
   Scenario: Rendered collection
     Given I have an "index.html" page that contains "Collections: {{ site.collections }}"
+    And I have an "collection_metadata.html" page that contains "Methods metadata: {{ site.collections.methods.foo }} {{ site.collections.methods }}"
     And I have fixture collections
-    And I have a configuration file with:
+    And I have a "_config.yml" file with content:
-      | key         | value       |
+    """
-      | collections | ['methods'] |
+    collections:
-      | render      | ['methods'] |
+      methods:
+        output: true
+        foo:   bar
+    """
     When I run jekyll build
     Then the _site directory should exist
-    And I should see "Collections: methods" in "_site/index.html"
+    And I should see "Collections: {\"methods" in "_site/index.html"
+    And I should see "Methods metadata: bar" in "_site/collection_metadata.html"
     And I should see "<p>Whatever: foo.bar</p>" in "_site/methods/configuration.html"
 
   Scenario: Rendered document in a layout
     Given I have an "index.html" page that contains "Collections: {{ site.collections }}"
     And I have a default layout that contains "<div class='title'>Tom Preston-Werner</div> {{content}}"
     And I have fixture collections
-    And I have a configuration file with:
+    And I have a "_config.yml" file with content:
-      | key         | value       |
+    """
-      | collections | ['methods'] |
+    collections:
-      | render      | ['methods'] |
+      methods:
+        output: true
+        foo:   bar
+    """
     When I run jekyll build
     Then the _site directory should exist
-    And I should see "Collections: methods" in "_site/index.html"
+    And I should see "Collections: {\"methods" in "_site/index.html"
     And I should see "<p>Run your generators! default</p>" in "_site/methods/site/generate.html"
-    And I should see "<div class='title'>Tom Preston-Werner</div>" in "_site/methods/site/generate.html"
+    And I should see "<div class='title'>Tom Preston-Werner</div>" in "_site/methods/site/generate.html"
+
+  Scenario: Collections specified as an array
+    Given I have an "index.html" page that contains "Collections: {% for method in site.methods %}{{ method.relative_path }} {% endfor %}"
+    And I have fixture collections
+    And I have a "_config.yml" file with content:
+    """
+    collections:
+    - methods
+    """
+    When I run jekyll build
+    Then the _site directory should exist
+    And I should see "Collections: _methods/configuration.md _methods/sanitized_path.md _methods/site/generate.md _methods/site/initialize.md _methods/um_hi.md" in "_site/index.html"
+
+  Scenario: Collections specified as an hash
+    Given I have an "index.html" page that contains "Collections: {% for method in site.methods %}{{ method.relative_path }} {% endfor %}"
+    And I have fixture collections
+    And I have a "_config.yml" file with content:
+    """
+    collections:
+      methods:
+        baz: bin
+    """
+    When I run jekyll build
+    Then the _site directory should exist
+    And I should see "Collections: _methods/configuration.md _methods/sanitized_path.md _methods/site/generate.md _methods/site/initialize.md _methods/um_hi.md" in "_site/index.html"

lib/jekyll/collection.rb

@@ -1,6 +1,6 @@
 module Jekyll
   class Collection
-    attr_reader :site, :label
+    attr_reader :site, :label, :metadata
 
     # Create a new Collection.
     #
@@ -9,8 +9,9 @@ module Jekyll
     #
     # Returns nothing.
     def initialize(site, label)
-      @site  = site
+      @site     = site
-      @label = sanitize_label(label)
+      @label    = sanitize_label(label)
+      @metadata = extract_metadata
     end
 
     # Fetch the Documents in this collection.
@@ -114,7 +115,32 @@ module Jekyll
     #
     # Returns a representation of this collection for use in Liquid.
     def to_liquid
-      docs
+      metadata.merge({
+        "label"     => label,
+        "docs"      => docs,
+        "directory" => directory,
+        "written"   => write?,
+        "relative_directory" => relative_directory
+      })
+    end
+
+    # Whether the collection's documents ought to be written as individual
+    #   files in the output.
+    #
+    # Returns true if the 'write' metadata is true, false otherwise.
+    def write?
+      !!metadata['output']
+    end
+
+    # Extract options for this collection from the site configuration.
+    #
+    # Returns the metadata for this collection
+    def extract_metadata
+      if site.config['collections'].is_a?(Hash)
+        site.config['collections'][label] || Hash.new
+      else
+        {}
+      end
     end
 
   end

lib/jekyll/document.rb

@@ -112,7 +112,7 @@ module Jekyll
     #
     # Returns the permalink or nil if no permalink was set in the data.
     def permalink
-      data && data['permalink']
+      data && data.is_a?(Hash) && data['permalink']
     end
 
     # The computed URL for the document. See `Jekyll::URL#to_s` for more details.
@@ -192,12 +192,16 @@ module Jekyll
     #
     # Returns a Hash representing this Document's data.
     def to_liquid
-      Utils.deep_merge_hashes data, {
+      if data.is_a?(Hash)
-        "content"       => content,
+        Utils.deep_merge_hashes data, {
-        "path"          => path,
+          "content"       => content,
-        "relative_path" => relative_path,
+          "path"          => path,
-        "url"           => url
+          "relative_path" => relative_path,
-      }
+          "url"           => url
+        }
+      else
+        data
+      end
     end
 
     # The inspect string for this document.

lib/jekyll/site.rb

@@ -4,7 +4,7 @@ module Jekyll
                   :exclude, :include, :source, :dest, :lsi, :highlighter,
                   :permalink_style, :time, :future, :unpublished, :safe, :plugins, :limit_posts,
                   :show_drafts, :keep_files, :baseurl, :data, :file_read_opts, :gems,
-                  :plugin_manager, :collections
+                  :plugin_manager
 
     attr_accessor :converters, :generators
 
@@ -55,6 +55,7 @@ module Jekyll
       self.pages = []
       self.static_files = []
       self.data = {}
+      @collections = nil
 
       if limit_posts < 0
         raise ArgumentError, "limit_posts must be a non-negative number"
@@ -90,18 +91,24 @@ module Jekyll
     #
     # Returns a Hash containing collection name-to-instance pairs.
     def collections
-      @collections ||= if config['collections']
+      @collections ||= Hash[collection_names.map { |coll| [coll, Jekyll::Collection.new(self, coll)] } ]
-        Hash[config['collections'].map { |coll| [coll, Jekyll::Collection.new(self, coll)] } ]
-      else
-        Hash.new
-      end
     end
 
-    # The list of collections to render.
+    # The list of collection names.
     #
-    # The array of collection labels to render.
+    # Returns an array of collection names from the configuration,
-    def to_render
+    #   or an empty array if the `collections` key is not set.
-      @to_render ||= (config['render'] || Array.new)
+    def collection_names
+      case config['collections']
+      when Hash
+        config['collections'].keys
+      when Array
+        config['collections']
+      when nil
+        []
+      else
+        raise ArgumentError, "Your `collections` key must be a hash or an array."
+      end
     end
 
     # Read Site data from disk and load it into internal data structures.
@@ -188,17 +195,18 @@ module Jekyll
     #
     # Returns nothing
     def read_data(dir)
-      unless dir.to_s.eql?("_data")
+      base = File.join(source, dir)
-        Jekyll.logger.error "Error:", "Data source directories other than '_data' have been removed.\n" +
+      return unless File.directory?(base) && (!safe || !File.symlink?(base))
-          "Please move your YAML files to `_data` and remove the `data_source` key from your `_config.yml`."
-      end
 
-      collections['data'] = Jekyll::Collection.new(self, "data")
+      entries = Dir.chdir(base) { Dir['*.{yaml,yml}'] }
-      collections['data'].read
+      entries.delete_if { |e| File.directory?(File.join(base, e)) }
 
-      collections['data'].docs.each do |doc|
+      entries.each do |entry|
-        key = sanitize_filename(doc.basename(".*"))
+        path = File.join(source, dir, entry)
-        self.data[key] = doc.data
+        next if File.symlink?(path) && safe
+
+        key = sanitize_filename(File.basename(entry, '.*'))
+        self.data[key] = SafeYAML.load_file(path)
       end
     end
 
@@ -206,7 +214,9 @@ module Jekyll
     #
     # Returns nothing.
     def read_collections
-      collections.each { |_, collection| collection.read }
+      collections.each do |_, collection|
+        collection.read unless collection.label.eql?("data")
+      end
     end
 
     # Run each of the Generators.
@@ -224,8 +234,8 @@ module Jekyll
     def render
       relative_permalinks_deprecation_method
 
-      to_render.each do |label|
+      collections.each do |label, collection|
-        collections[label].docs.each do |document|
+        collection.docs.each do |document|
           document.output = Jekyll::Renderer.new(self, document).run
         end
       end
@@ -306,7 +316,7 @@ module Jekyll
     def site_payload
       {"jekyll" => { "version" => Jekyll::VERSION },
        "site"   => Utils.deep_merge_hashes(config,
-         Utils.deep_merge_hashes(collections, {
+         Utils.deep_merge_hashes(Hash[collections.map{|label, coll| [label, coll.docs]}], {
           "time"         => time,
           "posts"        => posts.sort { |a, b| b <=> a },
           "pages"        => pages,
@@ -314,6 +324,7 @@ module Jekyll
           "html_pages"   => pages.reject { |page| !page.html? },
           "categories"   => post_attr_hash('categories'),
           "tags"         => post_attr_hash('tags'),
+          "collections"  => collections,
           "data"         => site_data
         }))
       }
@@ -395,9 +406,9 @@ module Jekyll
     end
 
     def documents
-      collections.reduce(Set.new) do |docs, (label, coll)|
+      collections.reduce(Set.new) do |docs, (_, collection)|
-        if to_render.include?(label)
+        if collection.write?
-          docs.merge(coll.docs)
+          docs.merge(collection.docs)
         else
           docs
         end

script/test

@@ -0,0 +1,11 @@
+#! /bin/bash
+
+set -x
+
+if [ -z "$1" ]; then
+  TEST_FILES="test/test*.rb"
+else
+  TEST_FILES="$@"
+fi
+
+/usr/bin/env bundle exec ruby -I"lib:test" -r rake -r rake/rake_test_loader ${TEST_FILES}

test/test_collections.rb

@@ -49,6 +49,35 @@ class TestCollections < Test::Unit::TestCase
     should "know the full path to itself on the filesystem" do
       assert_equal @collection.directory, source_dir("_methods")
     end
+
+    context "when turned into Liquid" do
+      should "have a label attribute" do
+        assert_equal @collection.to_liquid["label"], "methods"
+      end
+
+      should "have a docs attribute" do
+        assert_equal @collection.to_liquid["docs"], Array.new
+      end
+
+      should "have a directory attribute" do
+        assert_equal @collection.to_liquid["directory"], source_dir("_methods")
+      end
+
+      should "have a relative_directory attribute" do
+        assert_equal @collection.to_liquid["relative_directory"], "_methods"
+      end
+
+      should "have a written attribute" do
+        assert_equal @collection.to_liquid["written"], false
+      end
+    end
+
+    should "know whether it should be written or not" do
+      assert_equal @collection.write?, false
+      @collection.metadata['output'] = true
+      assert_equal @collection.write?, true
+      @collection.metadata.delete 'output'
+    end
   end
 
   context "with no collections specified" do
@@ -57,10 +86,8 @@ class TestCollections < Test::Unit::TestCase
       @site.process
     end
 
-    should "not contain any collections other than the default ones" do
+    should "not contain any collections" do
-      collections = @site.collections.dup
+      assert_equal Hash.new, @site.collections
-      assert collections.delete("data").is_a?(Jekyll::Collection)
-      assert_equal Hash.new, collections
     end
   end
 
@@ -107,6 +134,25 @@ class TestCollections < Test::Unit::TestCase
     end
   end
 
+  context "with a collection with metadata" do
+    setup do
+      @site = fixture_site({
+        "collections" => {
+          "methods" => {
+            "foo" => "bar",
+            "baz" => "whoo"
+          }
+        }
+      })
+      @site.process
+      @collection = @site.collections["methods"]
+    end
+
+    should "extract the configuration collection information as metadata" do
+      assert_equal @collection.metadata, {"foo" => "bar", "baz" => "whoo"}
+    end
+  end
+
   context "in safe mode" do
     setup do
       @site = fixture_site({