From db3aea1a1a312de43f4e5a62217a79f19ee8e7e8 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Thu, 23 Apr 2020 14:42:05 +0530 Subject: [PATCH 1/6] Allow disabling import of theme configuration --- docs/_docs/configuration/options.md | 18 ++++++++++++++++++ features/theme_configuration.feature | 11 +++++++++++ lib/jekyll/site.rb | 2 ++ 3 files changed, 31 insertions(+) diff --git a/docs/_docs/configuration/options.md b/docs/_docs/configuration/options.md index 90bfaff1478..d24da86b509 100644 --- a/docs/_docs/configuration/options.md +++ b/docs/_docs/configuration/options.md @@ -72,6 +72,24 @@ class="flag">flags (specified on the command-line) that control them.

--disable-disk-cache

+ + +

+ Ignore theme configuration + 4.1.0 +

+

+ Jekyll 4.0 started allowing themes to bundle a _config.yml + to simplify theme-onboarding for new users. + In the unfortunate situation that importing a bundled theme configuration + messes up the merged site-configuration, the user can configure Jekyll + to not import the theme-config entirely. +

+ + +

ignore_theme_config: BOOL

+ +

Exclude

diff --git a/features/theme_configuration.feature b/features/theme_configuration.feature index 6ddfd4ae662..00ed81ae4cc 100644 --- a/features/theme_configuration.feature +++ b/features/theme_configuration.feature @@ -11,6 +11,17 @@ Feature: Bundling Config file with Theme gems And the _site directory should exist And I should see "aero" in "_site/index.html" + Scenario: Disabling import of theme configuration entirely + Given I have a configuration file with: + | key | value | + | theme | test-theme | + | ignore_theme_config | true | + And I have an "index.md" page that contains "{{ site.test_theme.skin }}" + When I run jekyll build + Then I should get a zero exit status + And the _site directory should exist + And I should not see "aero" in "_site/index.html" + Scenario: A pre-configured theme with valid config file overriding Jekyll defaults Given I have a configuration file with "theme" set to "test-theme" And I have an "index.md" page that contains "{{ site.baseurl }}" diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index 5195a377e1e..c392a1fa248 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -432,6 +432,8 @@ def collections_path private def load_theme_configuration(config) + return config if config["ignore_theme_config"] + theme_config_file = in_theme_dir("_config.yml") return config unless File.exist?(theme_config_file) From a6f932e52e1755177b46e107a2ae32bbd177180e Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Thu, 30 Apr 2020 12:33:03 +0530 Subject: [PATCH 2/6] Disable import of theme config by default Users can *opt-in* by setting `import_theme_config: true` in their config file. --- features/theme_configuration.feature | 14 +++----------- lib/jekyll/site.rb | 2 +- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/features/theme_configuration.feature b/features/theme_configuration.feature index 00ed81ae4cc..0c8da0168c5 100644 --- a/features/theme_configuration.feature +++ b/features/theme_configuration.feature @@ -3,24 +3,16 @@ Feature: Bundling Config file with Theme gems I want to be able to pre-configure my gemified theme In order to make it easier for other Jekyllites to use my theme - Scenario: Easy onboarding with a pre-configured theme - Given I have a configuration file with "theme" set to "test-theme" - And I have an "index.md" page that contains "{{ site.test_theme.skin }}" - When I run jekyll build - Then I should get a zero exit status - And the _site directory should exist - And I should see "aero" in "_site/index.html" - - Scenario: Disabling import of theme configuration entirely + Scenario: Enabling import of theme configuration Given I have a configuration file with: | key | value | | theme | test-theme | - | ignore_theme_config | true | + | import_theme_config | true | And I have an "index.md" page that contains "{{ site.test_theme.skin }}" When I run jekyll build Then I should get a zero exit status And the _site directory should exist - And I should not see "aero" in "_site/index.html" + And I should see "aero" in "_site/index.html" Scenario: A pre-configured theme with valid config file overriding Jekyll defaults Given I have a configuration file with "theme" set to "test-theme" diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index c392a1fa248..b0969865288 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -432,7 +432,7 @@ def collections_path private def load_theme_configuration(config) - return config if config["ignore_theme_config"] + return config unless config["import_theme_config"] == true theme_config_file = in_theme_dir("_config.yml") return config unless File.exist?(theme_config_file) From 7b57b478137da2c3ac4620ff1ef6000817e5731e Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Thu, 30 Apr 2020 14:42:37 +0530 Subject: [PATCH 3/6] Revert changes made to test file --- test/test_site.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/test_site.rb b/test/test_site.rb index 122490e81d3..4ef9a1cb0a3 100644 --- a/test/test_site.rb +++ b/test/test_site.rb @@ -595,11 +595,7 @@ def convert(*_args) should "set a theme if the config is a string" do [:debug, :info, :warn, :error].each do |level| - if level == :info - expect(Jekyll.logger.writer).to receive(level) - else - expect(Jekyll.logger.writer).not_to receive(level) - end + expect(Jekyll.logger.writer).not_to receive(level) end site = fixture_site("theme" => "test-theme") assert_instance_of Jekyll::Theme, site.theme From d93ae60a76da0ef7d4950d23b37ec505a810fbea Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Fri, 1 May 2020 00:47:53 +0530 Subject: [PATCH 4/6] Revert "Revert changes made to test file" This reverts commit 7b57b478137da2c3ac4620ff1ef6000817e5731e. --- test/test_site.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/test_site.rb b/test/test_site.rb index 4ef9a1cb0a3..122490e81d3 100644 --- a/test/test_site.rb +++ b/test/test_site.rb @@ -595,7 +595,11 @@ def convert(*_args) should "set a theme if the config is a string" do [:debug, :info, :warn, :error].each do |level| - expect(Jekyll.logger.writer).not_to receive(level) + if level == :info + expect(Jekyll.logger.writer).to receive(level) + else + expect(Jekyll.logger.writer).not_to receive(level) + end end site = fixture_site("theme" => "test-theme") assert_instance_of Jekyll::Theme, site.theme From 01d1788d84f52b9ea2b50d8a4fa61ca3956911a5 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Fri, 1 May 2020 00:48:14 +0530 Subject: [PATCH 5/6] Revert "Disable import of theme config by default" This reverts commit a6f932e52e1755177b46e107a2ae32bbd177180e. --- features/theme_configuration.feature | 14 +++++++++++--- lib/jekyll/site.rb | 2 +- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/features/theme_configuration.feature b/features/theme_configuration.feature index 0c8da0168c5..00ed81ae4cc 100644 --- a/features/theme_configuration.feature +++ b/features/theme_configuration.feature @@ -3,16 +3,24 @@ Feature: Bundling Config file with Theme gems I want to be able to pre-configure my gemified theme In order to make it easier for other Jekyllites to use my theme - Scenario: Enabling import of theme configuration + Scenario: Easy onboarding with a pre-configured theme + Given I have a configuration file with "theme" set to "test-theme" + And I have an "index.md" page that contains "{{ site.test_theme.skin }}" + When I run jekyll build + Then I should get a zero exit status + And the _site directory should exist + And I should see "aero" in "_site/index.html" + + Scenario: Disabling import of theme configuration entirely Given I have a configuration file with: | key | value | | theme | test-theme | - | import_theme_config | true | + | ignore_theme_config | true | And I have an "index.md" page that contains "{{ site.test_theme.skin }}" When I run jekyll build Then I should get a zero exit status And the _site directory should exist - And I should see "aero" in "_site/index.html" + And I should not see "aero" in "_site/index.html" Scenario: A pre-configured theme with valid config file overriding Jekyll defaults Given I have a configuration file with "theme" set to "test-theme" diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index b0969865288..c392a1fa248 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -432,7 +432,7 @@ def collections_path private def load_theme_configuration(config) - return config unless config["import_theme_config"] == true + return config if config["ignore_theme_config"] theme_config_file = in_theme_dir("_config.yml") return config unless File.exist?(theme_config_file) From d21ddef65903ee5fd0ed19e4525c7f1fce9c012a Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Fri, 1 May 2020 00:53:46 +0530 Subject: [PATCH 6/6] Check for a Boolean true instead of a truthy --- 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 c392a1fa248..da8dce68d3c 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -432,7 +432,7 @@ def collections_path private def load_theme_configuration(config) - return config if config["ignore_theme_config"] + return config if config["ignore_theme_config"] == true theme_config_file = in_theme_dir("_config.yml") return config unless File.exist?(theme_config_file)