From 2bd592077df3b7c827e081457aec710bc131e0c3 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Tue, 2 Aug 2016 17:11:40 -0700 Subject: [PATCH 1/5] Site#configure_theme: do not set theme unless it's a string Some previous ad-hoc 'themes' used this configuration option to store a hash of values. In that case, we should simply pretend we have no theme. --- lib/jekyll/site.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index 9492b693..daf0e3e5 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -424,7 +424,7 @@ module Jekyll private def configure_theme self.theme = nil - self.theme = Jekyll::Theme.new(config["theme"]) if config["theme"] + self.theme = Jekyll::Theme.new(config["theme"]) if config["theme"].is_a?(String) end private From b937757dceeb3e0099c8103122034e55a795c86a Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Sun, 7 Aug 2016 12:03:50 -0700 Subject: [PATCH 2/5] Site#configure_theme: warn in case the 'theme' config is not a string --- lib/jekyll/site.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index daf0e3e5..02110f97 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -424,7 +424,14 @@ module Jekyll private def configure_theme self.theme = nil - self.theme = Jekyll::Theme.new(config["theme"]) if config["theme"].is_a?(String) + return unless config["theme"] + + if config["theme"].is_a?(String) + self.theme = Jekyll::Theme.new(config["theme"]) + else + Jekyll.logger.warn "Theme:", + "value of 'theme' in config should be String, but got #{config["theme"].class}" + end end private From 4420c3b2af9c826fa3db3de4949e201e14988260 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Tue, 30 Aug 2016 11:58:21 -0700 Subject: [PATCH 3/5] Make Site#configure_theme more understandable --- lib/jekyll/site.rb | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index 02110f97..91f58e3a 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -423,15 +423,14 @@ module Jekyll private def configure_theme - self.theme = nil - return unless config["theme"] - - if config["theme"].is_a?(String) - self.theme = Jekyll::Theme.new(config["theme"]) - else - Jekyll.logger.warn "Theme:", - "value of 'theme' in config should be String, but got #{config["theme"].class}" - end + self.theme = + if config["theme"].is_a?(String) + Jekyll::Theme.new(config["theme"]) + else + Jekyll.logger.warn "Theme:", "value of 'theme' in config should be " + "String to use gem-based themes, but got #{config["theme"].class}" + nil + end end private From 2b15b0b3251d35c290dc96eb07e18fa31a58bcc6 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Tue, 30 Aug 2016 12:17:24 -0700 Subject: [PATCH 4/5] Site#configure_theme: don't do anything if theme config is unset; TEST --- lib/jekyll/site.rb | 5 ++++- test/test_site.rb | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index 91f58e3a..66136541 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -423,11 +423,14 @@ module Jekyll private def configure_theme + self.theme = nil + return if config["theme"].nil? + self.theme = if config["theme"].is_a?(String) Jekyll::Theme.new(config["theme"]) else - Jekyll.logger.warn "Theme:", "value of 'theme' in config should be " + Jekyll.logger.warn "Theme:", "value of 'theme' in config should be " \ "String to use gem-based themes, but got #{config["theme"].class}" nil end diff --git a/test/test_site.rb b/test/test_site.rb index f09d94b9..615f8a69 100644 --- a/test/test_site.rb +++ b/test/test_site.rb @@ -489,6 +489,32 @@ class TestSite < JekyllUnitTest end end + context "when setting theme" do + should "set no theme if config is not set" do + expect($stderr).not_to receive(:puts) + expect($stdout).not_to receive(:puts) + site = fixture_site({ "theme" => nil }) + assert_nil site.theme + end + + should "set no theme if config is a hash" do + output = capture_output do + site = fixture_site({ "theme" => {} }) + assert_nil site.theme + end + expected_msg = "Theme: value of 'theme' in config should be String to use gem-based themes, but got Hash\n" + assert output.end_with?(expected_msg), "Expected #{output.inspect} to end with #{expected_msg.inspect}" + end + + should "set a theme if the config is a string" do + expect($stderr).not_to receive(:puts) + expect($stdout).not_to receive(:puts) + site = fixture_site({ "theme" => "test-theme" }) + assert_instance_of Jekyll::Theme, site.theme + assert_equal "test-theme", site.theme.name + end + end + context "with liquid profiling" do setup do @site = Site.new(site_configuration("profile" => true)) From 5b21f8fda9357ddf42b665dda5bf38e657b043b4 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Tue, 30 Aug 2016 13:28:46 -0700 Subject: [PATCH 5/5] Fix fmt errors. --- test/test_site.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/test_site.rb b/test/test_site.rb index 615f8a69..57b06871 100644 --- a/test/test_site.rb +++ b/test/test_site.rb @@ -502,8 +502,10 @@ class TestSite < JekyllUnitTest site = fixture_site({ "theme" => {} }) assert_nil site.theme end - expected_msg = "Theme: value of 'theme' in config should be String to use gem-based themes, but got Hash\n" - assert output.end_with?(expected_msg), "Expected #{output.inspect} to end with #{expected_msg.inspect}" + expected_msg = "Theme: value of 'theme' in config should be String " \ + "to use gem-based themes, but got Hash\n" + assert output.end_with?(expected_msg), + "Expected #{output.inspect} to end with #{expected_msg.inspect}" end should "set a theme if the config is a string" do