From c69ca4c11eac808a3cc8ec28286fea8a9ce7d7f8 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Mon, 4 Apr 2016 11:49:10 -0700 Subject: [PATCH 01/19] Test#build_configs shouldn't overwrite default collections --- test/helper.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/test/helper.rb b/test/helper.rb index 3a3dafa2..37c51d58 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -85,9 +85,12 @@ class JekyllUnitTest < Minitest::Test Jekyll::Site.new(site_configuration(overrides)) end - def build_configs(overrides, base_hash = Jekyll::Configuration::DEFAULTS) + def default_configuration + Marshal.load(Marshal.dump(Jekyll::Configuration::DEFAULTS)) + end + + def build_configs(overrides, base_hash = default_configuration) Utils.deep_merge_hashes(base_hash, overrides) - .fix_common_issues.backwards_compatibilize.add_default_collections end def site_configuration(overrides = {}) @@ -97,7 +100,10 @@ class JekyllUnitTest < Minitest::Test })) build_configs({ "source" => source_dir - }, full_overrides) + }, full_overrides). + fix_common_issues. + backwards_compatibilize. + add_default_collections end def dest_dir(*subdirs) From f2263a11b73091f0b94b8c6fe6f467e0a96b2d3d Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Mon, 4 Apr 2016 11:49:24 -0700 Subject: [PATCH 02/19] Only write collections.posts.permalink if permalink is set. --- lib/jekyll/configuration.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/jekyll/configuration.rb b/lib/jekyll/configuration.rb index 87bfeb46..5b5bb2df 100644 --- a/lib/jekyll/configuration.rb +++ b/lib/jekyll/configuration.rb @@ -72,7 +72,7 @@ module Jekyll 'hard_wrap' => false, 'footnote_nr' => 1 } - }] + }].freeze # Public: Turn all keys into string # @@ -271,14 +271,20 @@ module Jekyll def add_default_collections config = clone + # It defaults to `{}`, so this is only if someone sets it to null manually. return config if config['collections'].nil? + # Ensure we have a hash. if config['collections'].is_a?(Array) config['collections'] = Hash[config['collections'].map { |c| [c, {}] }] end + + # Add posts. config['collections']['posts'] ||= {} config['collections']['posts']['output'] = true - config['collections']['posts']['permalink'] = style_to_permalink(config['permalink']) + if config['permalink'] + config['collections']['posts']['permalink'] ||= style_to_permalink(config['permalink']) + end config end From dbcbf809ff6cc7b0a33f231a42523f480acd1a90 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Mon, 4 Apr 2016 13:12:30 -0700 Subject: [PATCH 03/19] Refactor some tests to prevent manipulation of Jekyll::Config::DEFAULTS --- test/helper.rb | 33 +++++++++----- test/test_configuration.rb | 40 +++++++++++------ test/test_front_matter_defaults.rb | 70 +++++++++++++----------------- test/test_generated_site.rb | 17 ++------ test/test_site.rb | 14 +++--- 5 files changed, 87 insertions(+), 87 deletions(-) diff --git a/test/helper.rb b/test/helper.rb index 37c51d58..90c806a0 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -58,11 +58,32 @@ module Minitest::Assertions def refute_exist(filename, msg = nil) msg = message(msg) { "Expected '#{filename}' not to exist" } refute File.exist?(filename), msg +end + +module DirectoryHelpers + def dest_dir(*subdirs) + test_dir('dest', *subdirs) + end + + def source_dir(*subdirs) + test_dir('source', *subdirs) + end + + def test_dir(*subdirs) + File.join(File.dirname(__FILE__), *subdirs) end end class JekyllUnitTest < Minitest::Test include ::RSpec::Mocks::ExampleMethods + include DirectoryHelpers + extend DirectoryHelpers + + def mu_pp obj + s = obj.is_a?(Hash) ? JSON.pretty_generate(obj) : obj.inspect + s = s.encode Encoding.default_external if defined? Encoding + s + end def mocks_expect(*args) RSpec::Mocks::ExampleMethods::ExpectHost.instance_method(:expect)\ @@ -106,23 +127,11 @@ class JekyllUnitTest < Minitest::Test add_default_collections end - def dest_dir(*subdirs) - test_dir("dest", *subdirs) - end - - def source_dir(*subdirs) - test_dir("source", *subdirs) - end - def clear_dest FileUtils.rm_rf(dest_dir) FileUtils.rm_rf(source_dir(".jekyll-metadata")) end - def test_dir(*subdirs) - File.join(File.dirname(__FILE__), *subdirs) - end - def directory_with_contents(path) FileUtils.rm_rf(path) FileUtils.mkdir(path) diff --git a/test/test_configuration.rb b/test/test_configuration.rb index 608dfef0..33800d2f 100644 --- a/test/test_configuration.rb +++ b/test/test_configuration.rb @@ -1,7 +1,10 @@ require 'helper' class TestConfiguration < JekyllUnitTest - @@defaults = Jekyll::Configuration::DEFAULTS.add_default_collections.freeze + @@test_config = { + "source" => new(nil).source_dir, + "destination" => dest_dir + } context "#stringify_keys" do setup do @@ -154,27 +157,27 @@ class TestConfiguration < JekyllUnitTest end context "loading configuration" do setup do - @path = File.join(Dir.pwd, '_config.yml') + @path = source_dir('_config.yml') @user_config = File.join(Dir.pwd, "my_config_file.yml") end should "fire warning with no _config.yml" do allow(SafeYAML).to receive(:load_file).with(@path) { raise SystemCallError, "No such file or directory - #{@path}" } allow($stderr).to receive(:puts).with("Configuration file: none".yellow) - assert_equal @@defaults, Jekyll.configuration({}) + assert_equal site_configuration, Jekyll.configuration(@@test_config) end should "load configuration as hash" do allow(SafeYAML).to receive(:load_file).with(@path).and_return(Hash.new) allow($stdout).to receive(:puts).with("Configuration file: #{@path}") - assert_equal @@defaults, Jekyll.configuration({}) + assert_equal site_configuration, Jekyll.configuration(@@test_config) end should "fire warning with bad config" do allow(SafeYAML).to receive(:load_file).with(@path).and_return(Array.new) allow($stderr).to receive(:puts).and_return(("WARNING: ".rjust(20) + "Error reading configuration. Using defaults (and options).").yellow) allow($stderr).to receive(:puts).and_return("Configuration file: (INVALID) #{@path}".yellow) - assert_equal @@defaults, Jekyll.configuration({}) + assert_equal site_configuration, Jekyll.configuration(@@test_config) end should "fire warning when user-specified config file isn't there" do @@ -193,8 +196,8 @@ class TestConfiguration < JekyllUnitTest context "loading config from external file" do setup do @paths = { - :default => File.join(Dir.pwd, '_config.yml'), - :other => File.join(Dir.pwd, '_config.live.yml'), + :default => source_dir('_config.yml'), + :other => source_dir('_config.live.yml'), :toml => source_dir('_config.dev.toml'), :empty => "" } @@ -203,24 +206,31 @@ class TestConfiguration < JekyllUnitTest should "load default plus posts config if no config_file is set" do allow(SafeYAML).to receive(:load_file).with(@paths[:default]).and_return({}) allow($stdout).to receive(:puts).with("Configuration file: #{@paths[:default]}") - assert_equal @@defaults, Jekyll.configuration({}) + assert_equal site_configuration, Jekyll.configuration(@@test_config) end should "load different config if specified" do allow(SafeYAML).to receive(:load_file).with(@paths[:other]).and_return({"baseurl" => "http://wahoo.dev"}) allow($stdout).to receive(:puts).with("Configuration file: #{@paths[:other]}") - assert_equal Utils.deep_merge_hashes(@@defaults, { "baseurl" => "http://wahoo.dev" }), Jekyll.configuration({ "config" => @paths[:other] }) + Jekyll.configuration({ "config" => @paths[:other] }) + assert_equal \ + site_configuration({ "baseurl" => "http://wahoo.dev" }), + Jekyll.configuration(@@test_config.merge({ "config" => @paths[:other] })) end should "load default config if path passed is empty" do allow(SafeYAML).to receive(:load_file).with(@paths[:default]).and_return({}) allow($stdout).to receive(:puts).with("Configuration file: #{@paths[:default]}") - assert_equal @@defaults, Jekyll.configuration({ "config" => @paths[:empty] }) + assert_equal \ + site_configuration, + Jekyll.configuration(@@test_config.merge({ "config" => [@paths[:empty]] })) end should "successfully load a TOML file" do Jekyll.logger.log_level = :warn - assert_equal @@defaults.clone.merge({ "baseurl" => "/you-beautiful-blog-you", "title" => "My magnificent site, wut" }), Jekyll.configuration({ "config" => [@paths[:toml]] }) + assert_equal \ + site_configuration({ "baseurl" => "/you-beautiful-blog-you", "title" => "My magnificent site, wut" }), + Jekyll.configuration(@@test_config.merge({ "config" => [@paths[:toml]] })) Jekyll.logger.log_level = :info end @@ -233,7 +243,9 @@ class TestConfiguration < JekyllUnitTest allow($stdout).to receive(:puts).with("Configuration file: #{@paths[:default]}") allow($stdout).to receive(:puts).with("Configuration file: #{@paths[:other]}") allow($stdout).to receive(:puts).with("Configuration file: #{@paths[:toml]}") - assert_equal @@defaults, Jekyll.configuration({ "config" => [@paths[:default], @paths[:other], @paths[:toml]] }) + assert_equal \ + site_configuration, + Jekyll.configuration(@@test_config.merge({ "config" => [@paths[:default], @paths[:other], @paths[:toml]] })) end should "load multiple config files and last config should win" do @@ -241,7 +253,9 @@ class TestConfiguration < JekyllUnitTest allow(SafeYAML).to receive(:load_file).with(@paths[:other]).and_return({"baseurl" => "http://wahoo.dev"}) allow($stdout).to receive(:puts).with("Configuration file: #{@paths[:default]}") allow($stdout).to receive(:puts).with("Configuration file: #{@paths[:other]}") - assert_equal Utils.deep_merge_hashes(@@defaults, { "baseurl" => "http://wahoo.dev" }), Jekyll.configuration({ "config" => [@paths[:default], @paths[:other]] }) + assert_equal \ + site_configuration({ "baseurl" => "http://wahoo.dev" }), + Jekyll.configuration(@@test_config.merge({ "config" => [@paths[:default], @paths[:other]] })) end end end diff --git a/test/test_front_matter_defaults.rb b/test/test_front_matter_defaults.rb index f1b55ac1..c85314d2 100644 --- a/test/test_front_matter_defaults.rb +++ b/test/test_front_matter_defaults.rb @@ -3,11 +3,9 @@ require "helper" class TestFrontMatterDefaults < JekyllUnitTest context "A site with full front matter defaults" do setup do - @site = Site.new(Jekyll.configuration({ - "source" => source_dir, - "destination" => dest_dir, - "defaults" => [{ - "scope" => { + @site = fixture_site({ + "defaults" => [{ + "scope" => { "path" => "contacts", "type" => "page" }, @@ -15,7 +13,7 @@ class TestFrontMatterDefaults < JekyllUnitTest "key" => "val" } }] - })) + }) @site.process @affected = @site.pages.find { |page| page.relative_path == "/contacts/bar.html" } @not_affected = @site.pages.find { |page| page.relative_path == "about.html" } @@ -29,18 +27,16 @@ class TestFrontMatterDefaults < JekyllUnitTest context "A site with front matter type pages and an extension" do setup do - @site = Site.new(Jekyll.configuration({ - "source" => source_dir, - "destination" => dest_dir, - "defaults" => [{ - "scope" => { + @site = fixture_site({ + "defaults" => [{ + "scope" => { "path" => "index.html" }, "values" => { "key" => "val" } }] - })) + }) @site.process @affected = @site.pages.find { |page| page.relative_path == "index.html" } @@ -55,18 +51,17 @@ class TestFrontMatterDefaults < JekyllUnitTest context "A site with front matter defaults with no type" do setup do - @site = Site.new(Jekyll.configuration({ - "source" => source_dir, - "destination" => dest_dir, - "defaults" => [{ - "scope" => { + @site = fixture_site({ + "defaults" => [{ + "scope" => { "path" => "win" }, "values" => { "key" => "val" } }] - })) + }) + @site.process @affected = @site.posts.docs.find { |page| page.relative_path =~ %r!win\/! } @not_affected = @site.pages.find { |page| page.relative_path == "about.html" } @@ -80,18 +75,17 @@ class TestFrontMatterDefaults < JekyllUnitTest context "A site with front matter defaults with no path and a deprecated type" do setup do - @site = Site.new(Jekyll.configuration({ - "source" => source_dir, - "destination" => dest_dir, - "defaults" => [{ - "scope" => { + @site = fixture_site({ + "defaults" => [{ + "scope" => { "type" => "page" }, "values" => { "key" => "val" } }] - })) + }) + @site.process @affected = @site.pages @not_affected = @site.posts.docs @@ -106,18 +100,16 @@ class TestFrontMatterDefaults < JekyllUnitTest context "A site with front matter defaults with no path" do setup do - @site = Site.new(Jekyll.configuration({ - "source" => source_dir, - "destination" => dest_dir, - "defaults" => [{ - "scope" => { + @site = fixture_site({ + "defaults" => [{ + "scope" => { "type" => "pages" }, "values" => { "key" => "val" } }] - })) + }) @site.process @affected = @site.pages @not_affected = @site.posts.docs @@ -132,17 +124,15 @@ class TestFrontMatterDefaults < JekyllUnitTest context "A site with front matter defaults with no path or type" do setup do - @site = Site.new(Jekyll.configuration({ - "source" => source_dir, - "destination" => dest_dir, - "defaults" => [{ - "scope" => { + @site = fixture_site({ + "defaults" => [{ + "scope" => { }, "values" => { "key" => "val" } }] - })) + }) @site.process @affected = @site.pages @not_affected = @site.posts @@ -156,15 +146,13 @@ class TestFrontMatterDefaults < JekyllUnitTest context "A site with front matter defaults with no scope" do setup do - @site = Site.new(Jekyll.configuration({ - "source" => source_dir, - "destination" => dest_dir, - "defaults" => [{ + @site = fixture_site({ + "defaults" => [{ "values" => { "key" => "val" } }] - })) + }) @site.process @affected = @site.pages @not_affected = @site.posts diff --git a/test/test_generated_site.rb b/test/test_generated_site.rb index 0a24172c..6b676d93 100644 --- a/test/test_generated_site.rb +++ b/test/test_generated_site.rb @@ -4,8 +4,6 @@ class TestGeneratedSite < JekyllUnitTest context "generated sites" do setup do clear_dest - config = Jekyll::Configuration::DEFAULTS.merge({ "source" => source_dir, - "destination" => dest_dir }) @site = fixture_site(config) @site.process @@ -80,24 +78,15 @@ OUTPUT should "ensure limit posts is 0 or more" do assert_raises ArgumentError do clear_dest - config = Jekyll::Configuration::DEFAULTS.merge({ - "source" => source_dir, - "destination" => dest_dir, - "limit_posts" => -1 - }) - @site = fixture_site(config) + + @site = fixture_site("limit_posts" => -1) end end should "acceptable limit post is 0" do clear_dest - config = Jekyll::Configuration::DEFAULTS.merge({ - "source" => source_dir, - "destination" => dest_dir, - "limit_posts" => 0 - }) - assert Site.new(config), "Couldn't create a site with the given limit_posts." + assert fixture_site("limit_posts" => 0), "Couldn't create a site with limit_posts=0." end end end diff --git a/test/test_site.rb b/test/test_site.rb index bce20bfa..89f646ed 100644 --- a/test/test_site.rb +++ b/test/test_site.rb @@ -3,7 +3,7 @@ require 'helper' class TestSite < JekyllUnitTest context "configuring sites" do should "have an array for plugins by default" do - site = Site.new(Jekyll::Configuration::DEFAULTS) + site = Site.new default_configuration assert_equal [File.join(Dir.pwd, '_plugins')], site.plugins end @@ -13,32 +13,32 @@ class TestSite < JekyllUnitTest end should "have an array for plugins if passed as a string" do - site = Site.new(Jekyll::Configuration::DEFAULTS.merge({'plugins_dir' => '/tmp/plugins'})) + site = Site.new(build_configs({ 'plugins_dir' => '/tmp/plugins' })) assert_equal ['/tmp/plugins'], site.plugins end should "have an array for plugins if passed as an array" do - site = Site.new(Jekyll::Configuration::DEFAULTS.merge({'plugins_dir' => ['/tmp/plugins', '/tmp/otherplugins']})) + site = Site.new(build_configs({ 'plugins_dir' => ['/tmp/plugins', '/tmp/otherplugins'] })) assert_equal ['/tmp/plugins', '/tmp/otherplugins'], site.plugins end should "have an empty array for plugins if nothing is passed" do - site = Site.new(Jekyll::Configuration::DEFAULTS.merge({'plugins_dir' => []})) + site = Site.new(build_configs({ 'plugins_dir' => [] })) assert_equal [], site.plugins end should "have an empty array for plugins if nil is passed" do - site = Site.new(Jekyll::Configuration::DEFAULTS.merge({'plugins_dir' => nil})) + site = Site.new(build_configs({ 'plugins_dir' => nil })) assert_equal [], site.plugins end should "expose default baseurl" do - site = Site.new(Jekyll::Configuration::DEFAULTS) + site = Site.new(default_configuration) assert_equal Jekyll::Configuration::DEFAULTS['baseurl'], site.baseurl end should "expose baseurl passed in from config" do - site = Site.new(Jekyll::Configuration::DEFAULTS.merge({'baseurl' => '/blog'})) + site = Site.new(build_configs({ 'baseurl' => '/blog' })) assert_equal '/blog', site.baseurl end end From 04d44731190ea19afe5547ac5b0f7928d4af1381 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Mon, 4 Apr 2016 13:13:06 -0700 Subject: [PATCH 04/19] Use Marshal to duplicate configuration defaults to prevent manipulation --- lib/jekyll.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/jekyll.rb b/lib/jekyll.rb index e9f4547b..cee9905d 100644 --- a/lib/jekyll.rb +++ b/lib/jekyll.rb @@ -99,7 +99,7 @@ module Jekyll # # Returns the final configuration Hash. def configuration(override = {}) - config = Configuration[Configuration::DEFAULTS] + config = Configuration[Marshal.load(Marshal.dump(Configuration::DEFAULTS))] override = Configuration[override].stringify_keys unless override.delete('skip_config_files') config = config.read_config_files(config.config_files(override)) From 6eaa8e90f8fe8fff5bac0853b5a13690745b0cdd Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Mon, 4 Apr 2016 13:13:23 -0700 Subject: [PATCH 05/19] Don't read a config file if the filename is empty. --- lib/jekyll/configuration.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/jekyll/configuration.rb b/lib/jekyll/configuration.rb index 5b5bb2df..4c36341d 100644 --- a/lib/jekyll/configuration.rb +++ b/lib/jekyll/configuration.rb @@ -169,6 +169,7 @@ module Jekyll begin files.each do |config_file| + next if config_file.nil? or config_file.empty? new_config = read_config_file(config_file) configuration = Utils.deep_merge_hashes(configuration, new_config) end From 4e06f07ad436badfbc031175041c61319ba3da2e Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Mon, 4 Apr 2016 13:56:40 -0700 Subject: [PATCH 06/19] Add tests for Configuration#add_default_collections --- test/test_configuration.rb | 54 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/test_configuration.rb b/test/test_configuration.rb index 33800d2f..fb545629 100644 --- a/test/test_configuration.rb +++ b/test/test_configuration.rb @@ -258,4 +258,58 @@ class TestConfiguration < JekyllUnitTest Jekyll.configuration(@@test_config.merge({ "config" => [@paths[:default], @paths[:other]] })) end end + + context "#add_default_collections" do + should "not do anything if collections is nil" do + conf = Configuration[default_configuration].tap {|c| c['collections'] = nil } + assert_equal conf.add_default_collections, conf + assert_nil conf.add_default_collections['collections'] + end + + should "converts collections to a hash if an array" do + conf = Configuration[default_configuration].tap {|c| c['collections'] = ['docs'] } + assert_equal conf.add_default_collections, conf.merge({ + "collections" => { + "docs" => {}, + "posts" => { + "output" => true, + "permalink" => "/:categories/:year/:month/:day/:title.html" + }}}) + end + + should "force collections.posts.output = true" do + conf = Configuration[default_configuration].tap {|c| c['collections'] = {'posts' => {'output' => false}} } + assert_equal conf.add_default_collections, conf.merge({ + "collections" => { + "posts" => { + "output" => true, + "permalink" => "/:categories/:year/:month/:day/:title.html" + }}}) + end + + should "set collections.posts.permalink if it's not set" do + conf = Configuration[default_configuration] + assert_equal conf.add_default_collections, conf.merge({ + "collections" => { + "posts" => { + "output" => true, + "permalink" => "/:categories/:year/:month/:day/:title.html" + }}}) + end + + should "leave collections.posts.permalink alone if it is set" do + posts_permalink = "/:year/:title/" + conf = Configuration[default_configuration].tap do |c| + c['collections'] = { + "posts" => { "permalink" => posts_permalink } + } + end + assert_equal conf.add_default_collections, conf.merge({ + "collections" => { + "posts" => { + "output" => true, + "permalink" => posts_permalink + }}}) + end + end end From fab092fcece39345b090b7301511df4bbc16591a Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Mon, 4 Apr 2016 14:45:09 -0700 Subject: [PATCH 07/19] Remove use of Marshal in runtime code. --- lib/jekyll.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/jekyll.rb b/lib/jekyll.rb index cee9905d..e9f4547b 100644 --- a/lib/jekyll.rb +++ b/lib/jekyll.rb @@ -99,7 +99,7 @@ module Jekyll # # Returns the final configuration Hash. def configuration(override = {}) - config = Configuration[Marshal.load(Marshal.dump(Configuration::DEFAULTS))] + config = Configuration[Configuration::DEFAULTS] override = Configuration[override].stringify_keys unless override.delete('skip_config_files') config = config.read_config_files(config.config_files(override)) From d01f7943debf08dee645990ddb0e00d59cf846bb Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 8 Apr 2016 10:00:03 -0700 Subject: [PATCH 08/19] Add Configuration.from & use in Jekyll.configuration. This process streamlines the creation of new configurations. Creating a new site will choke if not all the correct options are given. Configuration.from will ensure the overrides have all string keys and ensures all the common issues & defaults are in place so a Site can be created. A common use: config = Configuration.from({ 'permalink' => '/:title/' }) # etc site = Jekyll::Site.new(config) --- lib/jekyll.rb | 7 +++---- lib/jekyll/configuration.rb | 19 ++++++++++++++++++- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/lib/jekyll.rb b/lib/jekyll.rb index e9f4547b..0d680842 100644 --- a/lib/jekyll.rb +++ b/lib/jekyll.rb @@ -98,15 +98,14 @@ module Jekyll # list of option names and their defaults. # # Returns the final configuration Hash. - def configuration(override = {}) - config = Configuration[Configuration::DEFAULTS] - override = Configuration[override].stringify_keys + def configuration(override = Hash.new) + config = Configuration.new unless override.delete('skip_config_files') config = config.read_config_files(config.config_files(override)) end # Merge DEFAULTS < _config.yml < override - config = Utils.deep_merge_hashes(config, override).stringify_keys + config = Configuration.from Utils.deep_merge_hashes(config, override).stringify_keys set_timezone(config['timezone']) if config['timezone'] config diff --git a/lib/jekyll/configuration.rb b/lib/jekyll/configuration.rb index 4c36341d..4af19a63 100644 --- a/lib/jekyll/configuration.rb +++ b/lib/jekyll/configuration.rb @@ -74,6 +74,23 @@ module Jekyll } }].freeze + class << self + # Static: Produce a Configuration ready for use in a Site. + # It takes the input, fills in the defaults where values do not + # exist, and patches common issues including migrating options for + # backwards compatiblity. Except where a key or value is being fixed, + # the user configuration will override the defaults. + # + # user_config - a Hash or Configuration of overrides. + # + # Returns a Configuration filled with defaults and fixed for common + # problems and backwards-compatibility. + def from(user_config) + Utils.deep_merge_hashes(DEFAULTS, Configuration[user_config].stringify_keys). + fix_common_issues.backwards_compatibilize.add_default_collections + end + end + # Public: Turn all keys into string # # Return a copy of the hash where all its keys are strings @@ -237,7 +254,7 @@ module Jekyll " as a list of comma-separated values." config[option] = csv_to_array(config[option]) end - config[option].map!(&:to_s) + config[option].map!(&:to_s) if config[option] end if (config['kramdown'] || {}).key?('use_coderay') From f52a0e7200957362d8078adc3c15cda303543e43 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 8 Apr 2016 10:49:08 -0700 Subject: [PATCH 09/19] Configuration#add_default_collections: fix bug where DEFAULTS['collections'] is modified --- lib/jekyll/configuration.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/jekyll/configuration.rb b/lib/jekyll/configuration.rb index 4af19a63..03547604 100644 --- a/lib/jekyll/configuration.rb +++ b/lib/jekyll/configuration.rb @@ -297,11 +297,13 @@ module Jekyll config['collections'] = Hash[config['collections'].map { |c| [c, {}] }] end - # Add posts. - config['collections']['posts'] ||= {} - config['collections']['posts']['output'] = true - if config['permalink'] - config['collections']['posts']['permalink'] ||= style_to_permalink(config['permalink']) + config['collections'] = Utils.deep_merge_hashes( + { 'posts' => {} }, config['collections'] + ).tap do |collections| + collections['posts']['output'] = true + if config['permalink'] + collections['posts']['permalink'] ||= style_to_permalink(config['permalink']) + end end config From 8af77643c5d63eb5b241f410e34e728eeeef71b2 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 8 Apr 2016 10:50:58 -0700 Subject: [PATCH 10/19] Site#site_payload: sort collections by label --- lib/jekyll/drops/site_drop.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/jekyll/drops/site_drop.rb b/lib/jekyll/drops/site_drop.rb index 4d07ebe8..7b083126 100644 --- a/lib/jekyll/drops/site_drop.rb +++ b/lib/jekyll/drops/site_drop.rb @@ -28,7 +28,7 @@ module Jekyll end def collections - @site_collections ||= @obj.collections.values.map(&:to_liquid) + @site_collections ||= @obj.collections.values.sort_by(&:label).map(&:to_liquid) end private From 59346eb2285e9b122c0efabbbe03e715382b7723 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Tue, 26 Apr 2016 17:02:16 -0700 Subject: [PATCH 11/19] Remove call to #backwards_compatibilize in Configuration.from --- lib/jekyll/configuration.rb | 2 +- test/test_configuration.rb | 52 +++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/lib/jekyll/configuration.rb b/lib/jekyll/configuration.rb index 03547604..a08c284c 100644 --- a/lib/jekyll/configuration.rb +++ b/lib/jekyll/configuration.rb @@ -87,7 +87,7 @@ module Jekyll # problems and backwards-compatibility. def from(user_config) Utils.deep_merge_hashes(DEFAULTS, Configuration[user_config].stringify_keys). - fix_common_issues.backwards_compatibilize.add_default_collections + fix_common_issues.add_default_collections end end diff --git a/test/test_configuration.rb b/test/test_configuration.rb index fb545629..6322835d 100644 --- a/test/test_configuration.rb +++ b/test/test_configuration.rb @@ -6,6 +6,58 @@ class TestConfiguration < JekyllUnitTest "destination" => dest_dir } + context ".from" do + should "create a Configuration object" do + assert_instance_of Configuration, Configuration.from({}) + end + + should "merge input over defaults" do + result = Configuration.from({"source" => "blah"}) + refute_equal result["source"], Configuration::DEFAULTS["source"] + assert_equal result["source"], "blah" + end + + should "fix common mistakes" do + result = Configuration.from({"paginate" => 0}) + assert_nil result["paginate"], "Expected 'paginate' to be corrected to 'nil', but was #{result["paginate"].inspect}" + end + + should "add default collections" do + result = Configuration.from({}) + assert_equal result["collections"], {"posts" => {"output" => true, "permalink" => "/:categories/:year/:month/:day/:title.html"}} + end + + should "NOT backwards-compatibilize" do + assert Configuration.from("watch" => true)["watch"], "Expected the 'watch' key to not be removed." + end + end + + context "#add_default_collections" do + should "no-op if collections is nil" do + result = Configuration[{"collections" => nil}].add_default_collections + assert_nil result["collections"] + end + + should "turn an array into a hash" do + result = Configuration[{"collections" => %w{methods}}].add_default_collections + assert_instance_of Hash, result["collections"] + assert_equal result["collections"], {"posts" => {"output" => true}, "methods" => {}} + end + + should "only assign collections.posts.permalink if a permalink is specified" do + result = Configuration[{"permalink" => "pretty", "collections" => {}}].add_default_collections + assert_equal result["collections"], {"posts" => {"output" => true, "permalink" => "/:categories/:year/:month/:day/:title/"}} + + result = Configuration[{"permalink" => nil, "collections" => {}}].add_default_collections + assert_equal result["collections"], {"posts" => {"output" => true}} + end + + should "forces posts to output" do + result = Configuration[{"collections" => {"posts" => {"output" => false}}}].add_default_collections + assert_equal result["collections"]["posts"]["output"], true + end + end + context "#stringify_keys" do setup do @mixed_keys = Configuration[{ From 37b93f10dd880d6561037c916bf5571f3296f4d3 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Wed, 18 May 2016 11:31:22 -0700 Subject: [PATCH 12/19] Add missing 'end' to test/helper.rb --- test/helper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/helper.rb b/test/helper.rb index 90c806a0..5668f2ec 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -58,6 +58,7 @@ module Minitest::Assertions def refute_exist(filename, msg = nil) msg = message(msg) { "Expected '#{filename}' not to exist" } refute File.exist?(filename), msg + end end module DirectoryHelpers From ad59b6e62a68219ee82f3df4f45b8cb375e77457 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Wed, 18 May 2016 11:36:57 -0700 Subject: [PATCH 13/19] Remove merge conflicts I forgot to fix. --- test/test_generated_site.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/test/test_generated_site.rb b/test/test_generated_site.rb index 6b676d93..0b425b13 100644 --- a/test/test_generated_site.rb +++ b/test/test_generated_site.rb @@ -5,7 +5,7 @@ class TestGeneratedSite < JekyllUnitTest setup do clear_dest - @site = fixture_site(config) + @site = fixture_site @site.process @index = File.read(dest_dir("index.html")) end @@ -63,10 +63,7 @@ OUTPUT context "generating limited posts" do setup do clear_dest - config = Jekyll::Configuration::DEFAULTS.merge({ "source" => source_dir, - "destination" => dest_dir, - "limit_posts" => 5 }) - @site = fixture_site(config) + @site = fixture_site("limit_posts" => 5) @site.process @index = File.read(dest_dir("index.html")) end @@ -78,14 +75,12 @@ OUTPUT should "ensure limit posts is 0 or more" do assert_raises ArgumentError do clear_dest - @site = fixture_site("limit_posts" => -1) end end should "acceptable limit post is 0" do clear_dest - assert fixture_site("limit_posts" => 0), "Couldn't create a site with limit_posts=0." end end From d84844c2230948082de7a9270023f732a4542bb0 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Wed, 18 May 2016 12:48:42 -0700 Subject: [PATCH 14/19] Freeze configuration defaults & duplicate in deep_merge_hashes if need be. --- lib/jekyll/configuration.rb | 2 +- lib/jekyll/utils.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/jekyll/configuration.rb b/lib/jekyll/configuration.rb index a08c284c..475aa4e6 100644 --- a/lib/jekyll/configuration.rb +++ b/lib/jekyll/configuration.rb @@ -72,7 +72,7 @@ module Jekyll 'hard_wrap' => false, 'footnote_nr' => 1 } - }].freeze + }.map { |k, v| [k, v.freeze] }].freeze class << self # Static: Produce a Configuration ready for use in a Site. diff --git a/lib/jekyll/utils.rb b/lib/jekyll/utils.rb index 179981ec..a70a7916 100644 --- a/lib/jekyll/utils.rb +++ b/lib/jekyll/utils.rb @@ -54,6 +54,10 @@ module Jekyll target.default_proc = overwrite.default_proc end + target.each do |key, val| + target[key] = val.dup if val.frozen? && duplicable?(val) + end + target end @@ -61,6 +65,15 @@ module Jekyll value.is_a?(Hash) || value.is_a?(Drops::Drop) end + def duplicable?(obj) + case obj + when nil, false, true, Symbol, Numeric + false + else + true + end + end + # Read array from the supplied hash favouring the singular key # and then the plural key, and handling any nil entries. # From de5970ae55503b0c6fb6bf72464d7fe093b306e3 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Wed, 18 May 2016 12:49:06 -0700 Subject: [PATCH 15/19] Fix some minor things in the tests --- lib/jekyll.rb | 7 +++---- test/test_configuration.rb | 8 ++++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/jekyll.rb b/lib/jekyll.rb index 0d680842..07863517 100644 --- a/lib/jekyll.rb +++ b/lib/jekyll.rb @@ -105,10 +105,9 @@ module Jekyll end # Merge DEFAULTS < _config.yml < override - config = Configuration.from Utils.deep_merge_hashes(config, override).stringify_keys - set_timezone(config['timezone']) if config['timezone'] - - config + Configuration.from(Utils.deep_merge_hashes(config, override)).tap do |config| + set_timezone(config['timezone']) if config['timezone'] + end end # Public: Set the TZ environment variable to use the timezone specified diff --git a/test/test_configuration.rb b/test/test_configuration.rb index 6322835d..08cc3bd9 100644 --- a/test/test_configuration.rb +++ b/test/test_configuration.rb @@ -24,7 +24,7 @@ class TestConfiguration < JekyllUnitTest should "add default collections" do result = Configuration.from({}) - assert_equal result["collections"], {"posts" => {"output" => true, "permalink" => "/:categories/:year/:month/:day/:title.html"}} + assert_equal result["collections"], {"posts" => {"output" => true, "permalink" => "/:categories/:year/:month/:day/:title:output_ext"}} end should "NOT backwards-compatibilize" do @@ -325,7 +325,7 @@ class TestConfiguration < JekyllUnitTest "docs" => {}, "posts" => { "output" => true, - "permalink" => "/:categories/:year/:month/:day/:title.html" + "permalink" => "/:categories/:year/:month/:day/:title:output_ext" }}}) end @@ -335,7 +335,7 @@ class TestConfiguration < JekyllUnitTest "collections" => { "posts" => { "output" => true, - "permalink" => "/:categories/:year/:month/:day/:title.html" + "permalink" => "/:categories/:year/:month/:day/:title:output_ext" }}}) end @@ -345,7 +345,7 @@ class TestConfiguration < JekyllUnitTest "collections" => { "posts" => { "output" => true, - "permalink" => "/:categories/:year/:month/:day/:title.html" + "permalink" => "/:categories/:year/:month/:day/:title:output_ext" }}}) end From d5c3785d293bd739c9439c35eada422442c3e4e4 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Wed, 18 May 2016 12:49:23 -0700 Subject: [PATCH 16/19] Don't default 'include' and 'exclude' to an empty array --- lib/jekyll/configuration.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/jekyll/configuration.rb b/lib/jekyll/configuration.rb index 475aa4e6..a88272cc 100644 --- a/lib/jekyll/configuration.rb +++ b/lib/jekyll/configuration.rb @@ -246,7 +246,6 @@ module Jekyll end %w(include exclude).each do |option| - config[option] ||= [] if config[option].is_a?(String) Jekyll::Deprecator.deprecation_message "The '#{option}' configuration option" \ " must now be specified as an array, but you specified" \ From 48274244e39c5574a8df24648a911fe672e3dcee Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Wed, 18 May 2016 12:53:08 -0700 Subject: [PATCH 17/19] Define Drop#each so we can use the new frozen/duping behavior --- lib/jekyll/drops/drop.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/jekyll/drops/drop.rb b/lib/jekyll/drops/drop.rb index d1bffcc5..7dc829d7 100644 --- a/lib/jekyll/drops/drop.rb +++ b/lib/jekyll/drops/drop.rb @@ -147,6 +147,12 @@ module Jekyll keys.each(&block) end + def each(&block) + each_key.each do |key| + yield key, self[key] + end + end + def merge(other, &block) self.dup.tap do |me| if block.nil? From 7641971d7e355a8041bec96b8ce843b89f5daebc Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Wed, 18 May 2016 14:49:21 -0700 Subject: [PATCH 18/19] Fix tests for plugins in configuration. --- lib/jekyll/plugin_manager.rb | 2 +- test/test_site.rb | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/jekyll/plugin_manager.rb b/lib/jekyll/plugin_manager.rb index 45eeaa79..5acd0e46 100644 --- a/lib/jekyll/plugin_manager.rb +++ b/lib/jekyll/plugin_manager.rb @@ -76,7 +76,7 @@ module Jekyll # # Returns an Array of plugin search paths def plugins_path - if site.config['plugins_dir'] == Jekyll::Configuration::DEFAULTS['plugins_dir'] + if site.config['plugins_dir'].eql? Jekyll::Configuration::DEFAULTS['plugins_dir'] [site.in_source_dir(site.config['plugins_dir'])] else Array(site.config['plugins_dir']).map { |d| File.expand_path(d) } diff --git a/test/test_site.rb b/test/test_site.rb index 89f646ed..5e52a7c9 100644 --- a/test/test_site.rb +++ b/test/test_site.rb @@ -9,27 +9,27 @@ class TestSite < JekyllUnitTest should "look for plugins under the site directory by default" do site = Site.new(site_configuration) - assert_equal [File.join(source_dir, '_plugins')], site.plugins + assert_equal [source_dir('_plugins')], site.plugins end should "have an array for plugins if passed as a string" do - site = Site.new(build_configs({ 'plugins_dir' => '/tmp/plugins' })) + site = Site.new(site_configuration({ 'plugins_dir' => '/tmp/plugins' })) assert_equal ['/tmp/plugins'], site.plugins end should "have an array for plugins if passed as an array" do - site = Site.new(build_configs({ 'plugins_dir' => ['/tmp/plugins', '/tmp/otherplugins'] })) + site = Site.new(site_configuration({ 'plugins_dir' => ['/tmp/plugins', '/tmp/otherplugins'] })) assert_equal ['/tmp/plugins', '/tmp/otherplugins'], site.plugins end should "have an empty array for plugins if nothing is passed" do - site = Site.new(build_configs({ 'plugins_dir' => [] })) + site = Site.new(site_configuration({ 'plugins_dir' => [] })) assert_equal [], site.plugins end - should "have an empty array for plugins if nil is passed" do - site = Site.new(build_configs({ 'plugins_dir' => nil })) - assert_equal [], site.plugins + should "have the default for plugins if nil is passed" do + site = Site.new(site_configuration({ 'plugins_dir' => nil })) + assert_equal [source_dir('_plugins')], site.plugins end should "expose default baseurl" do @@ -38,7 +38,7 @@ class TestSite < JekyllUnitTest end should "expose baseurl passed in from config" do - site = Site.new(build_configs({ 'baseurl' => '/blog' })) + site = Site.new(site_configuration({ 'baseurl' => '/blog' })) assert_equal '/blog', site.baseurl end end From cc0c5ea19e8fa1e8bd3e46f09a2063adc277d79a Mon Sep 17 00:00:00 2001 From: Pat Hawks Date: Tue, 24 May 2016 19:01:35 -0500 Subject: [PATCH 19/19] Rubocop fixes --- test/helper.rb | 14 +++++++------- test/test_front_matter_defaults.rb | 12 ++++++------ test/test_generated_site.rb | 5 ++++- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/test/helper.rb b/test/helper.rb index 5668f2ec..751a14b1 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -63,11 +63,11 @@ end module DirectoryHelpers def dest_dir(*subdirs) - test_dir('dest', *subdirs) + test_dir("dest", *subdirs) end def source_dir(*subdirs) - test_dir('source', *subdirs) + test_dir("source", *subdirs) end def test_dir(*subdirs) @@ -80,7 +80,7 @@ class JekyllUnitTest < Minitest::Test include DirectoryHelpers extend DirectoryHelpers - def mu_pp obj + def mu_pp(obj) s = obj.is_a?(Hash) ? JSON.pretty_generate(obj) : obj.inspect s = s.encode Encoding.default_external if defined? Encoding s @@ -122,10 +122,10 @@ class JekyllUnitTest < Minitest::Test })) build_configs({ "source" => source_dir - }, full_overrides). - fix_common_issues. - backwards_compatibilize. - add_default_collections + }, full_overrides) + .fix_common_issues + .backwards_compatibilize + .add_default_collections end def clear_dest diff --git a/test/test_front_matter_defaults.rb b/test/test_front_matter_defaults.rb index c85314d2..fdad0930 100644 --- a/test/test_front_matter_defaults.rb +++ b/test/test_front_matter_defaults.rb @@ -5,7 +5,7 @@ class TestFrontMatterDefaults < JekyllUnitTest setup do @site = fixture_site({ "defaults" => [{ - "scope" => { + "scope" => { "path" => "contacts", "type" => "page" }, @@ -29,7 +29,7 @@ class TestFrontMatterDefaults < JekyllUnitTest setup do @site = fixture_site({ "defaults" => [{ - "scope" => { + "scope" => { "path" => "index.html" }, "values" => { @@ -53,7 +53,7 @@ class TestFrontMatterDefaults < JekyllUnitTest setup do @site = fixture_site({ "defaults" => [{ - "scope" => { + "scope" => { "path" => "win" }, "values" => { @@ -77,7 +77,7 @@ class TestFrontMatterDefaults < JekyllUnitTest setup do @site = fixture_site({ "defaults" => [{ - "scope" => { + "scope" => { "type" => "page" }, "values" => { @@ -102,7 +102,7 @@ class TestFrontMatterDefaults < JekyllUnitTest setup do @site = fixture_site({ "defaults" => [{ - "scope" => { + "scope" => { "type" => "pages" }, "values" => { @@ -126,7 +126,7 @@ class TestFrontMatterDefaults < JekyllUnitTest setup do @site = fixture_site({ "defaults" => [{ - "scope" => { + "scope" => { }, "values" => { "key" => "val" diff --git a/test/test_generated_site.rb b/test/test_generated_site.rb index 0b425b13..8373f0fc 100644 --- a/test/test_generated_site.rb +++ b/test/test_generated_site.rb @@ -81,7 +81,10 @@ OUTPUT should "acceptable limit post is 0" do clear_dest - assert fixture_site("limit_posts" => 0), "Couldn't create a site with limit_posts=0." + assert( + fixture_site("limit_posts" => 0), + "Couldn't create a site with limit_posts=0." + ) end end end