Merge pull request #772 from github/remove-gem-themes

Remove gem-based themes, replace with remote themes

Author: parkr

lib/github-pages/configuration.rb

@@ -107,6 +107,7 @@ def effective_config(user_config)
         # Allow theme to be explicitly disabled via "theme: null"
         config["theme"] = user_config["theme"] if user_config.key?("theme")
 
+        migrate_theme_to_remote_theme(config)
         exclude_cname(config)
 
         # Merge overwrites into user config
@@ -156,6 +157,16 @@ def restrict_and_config_markdown_processor(config)
         }
       end
 
+      # If the user has set a 'theme', then see if we can automatically use remote theme instead.
+      def migrate_theme_to_remote_theme(config)
+        return unless config["theme"]
+        return unless GitHubPages::Plugins::THEMES_TO_CONVERT_TO_REMOTE_THEMES.key?(config["theme"])
+
+        theme_name = config.delete("theme")
+        config["remote_theme"] ||= GitHubPages::Plugins::THEMES_TO_CONVERT_TO_REMOTE_THEMES[theme_name]
+        config["plugins"] = Array(config["plugins"]).concat(["jekyll-remote-theme"]).uniq
+      end
+
       # If the user's 'exclude' config is the default, also exclude the CNAME
       def exclude_cname(config)
         return unless config["exclude"].eql? Jekyll::Configuration::DEFAULTS["exclude"]

lib/github-pages/plugins.rb

@@ -48,21 +48,25 @@ class Plugins
 
     # Themes
     THEMES = {
-      "jekyll-swiss" => "1.0.0",
       "minima" => "2.5.1",
-      "jekyll-theme-primer" => "0.5.4",
-      "jekyll-theme-architect" => "0.1.1",
-      "jekyll-theme-cayman" => "0.1.1",
-      "jekyll-theme-dinky" => "0.1.1",
-      "jekyll-theme-hacker" => "0.1.2",
-      "jekyll-theme-leap-day" => "0.1.1",
-      "jekyll-theme-merlot" => "0.1.1",
-      "jekyll-theme-midnight" => "0.1.1",
-      "jekyll-theme-minimal" => "0.1.1",
-      "jekyll-theme-modernist" => "0.1.1",
-      "jekyll-theme-slate" => "0.1.1",
-      "jekyll-theme-tactile" => "0.1.1",
-      "jekyll-theme-time-machine" => "0.1.1",
     }.freeze
+
+    # Themes to convert to remote themes
+    THEMES_TO_CONVERT_TO_REMOTE_THEMES = {
+      "jekyll-swiss" => "broccolini/swiss",
+      "jekyll-theme-primer" => "pages-themes/[email protected]",
+      "jekyll-theme-architect" => "pages-themes/[email protected]",
+      "jekyll-theme-cayman" => "pages-themes/[email protected]",
+      "jekyll-theme-dinky" => "pages-themes/[email protected]",
+      "jekyll-theme-hacker" => "pages-themes/[email protected]",
+      "jekyll-theme-leap-day" => "pages-themes/[email protected]",
+      "jekyll-theme-merlot" => "pages-themes/[email protected]",
+      "jekyll-theme-midnight" => "pages-themes/[email protected]",
+      "jekyll-theme-minimal" => "pages-themes/[email protected]",
+      "jekyll-theme-modernist" => "pages-themes/[email protected]",
+      "jekyll-theme-slate" => "pages-themes/[email protected]",
+      "jekyll-theme-tactile" => "pages-themes/[email protected]",
+      "jekyll-theme-time-machine" => "pages-themes/[email protected]",
+    }
   end
 end

spec/github-pages/configuration_spec.rb

@@ -20,6 +20,7 @@
     ENV.delete("DISABLE_WHITELIST")
     ENV["JEKYLL_ENV"] = "test"
     ENV["PAGES_REPO_NWO"] = "github/pages-gem"
+    stub_request_for_remote_theme(:repo => "pages-themes/primer", :revision => "v0.5.4", :filename => "primer-0.5.4.zip")
   end
 
   context "#effective_config" do
@@ -103,22 +104,32 @@
     context "themes" do
       context "with no theme set" do
         it "sets the theme" do
+          expect(effective_config["plugins"]).to include("jekyll-remote-theme")
           expect(site.theme).to_not be_nil
           expect(site.theme).to be_a(Jekyll::Theme)
-          expect(site.theme.name).to eql("jekyll-theme-primer")
+          expect(site.theme.name).to eql("primer")
         end
       end
 
       context "with a user-specified theme" do
-        let(:site) do
-          config = configuration.merge("theme" => "jekyll-theme-merlot")
-          Jekyll::Site.new(config)
+        let(:configuration) do
+          described_class.effective_config(
+            Jekyll.configuration(
+              test_config.merge("theme" => "jekyll-theme-merlot")
+            )
+          )
+        end
+
+        before(:each) do
+          stub_request_for_remote_theme(:repo => "pages-themes/merlot", :revision => "v0.1.1", :filename => "merlot-0.1.1.zip")
         end
 
         it "respects the theme" do
+          expect(configuration["theme"]).to be_nil
+          expect(configuration["remote_theme"]).to eq(GitHubPages::Plugins::THEMES_TO_CONVERT_TO_REMOTE_THEMES["jekyll-theme-merlot"])
           expect(site.theme).to_not be_nil
           expect(site.theme).to be_a(Jekyll::Theme)
-          expect(site.theme.name).to eql("jekyll-theme-merlot")
+          expect(site.theme.name).to eql("merlot")
         end
       end
 
@@ -133,10 +144,6 @@
         end
       end
 
-      it "plugins don't include jekyll remote theme" do
-        expect(effective_config["plugins"]).to_not include("jekyll-remote-theme")
-      end
-
       context "with a remote theme" do
         let(:test_config) do
           {
@@ -149,6 +156,7 @@
         end
 
         it "plugins include jekyll remote theme" do
+          stub_request_for_remote_theme(:repo => "foo/bar", :revision => "HEAD", :filename => "primer-0.5.4.zip")
           expect(effective_config["plugins"]).to include("jekyll-remote-theme")
         end
       end

spec/github-pages/integration_spec.rb

@@ -258,7 +258,7 @@ def rm_destination
 
     context "jekyll-theme-primer" do
       it "sets the theme" do
-        expect(contents).to match("Theme: jekyll-theme-primer")
+        expect(contents).to match("Theme: primer")
       end
 
       it "uses the theme" do

spec/spec_helper.rb

@@ -29,6 +29,15 @@ def tmp_dir
   File.expand_path "../tmp", __dir__
 end
 
+def support_dir
+  File.expand_path "support", __dir__
+end
+
+def stub_request_for_remote_theme(repo:, revision:, filename:)
+  stub_request(:get, "https://codeload.github.com/#{repo}/zip/#{revision}").
+    to_return(:status => 200, :body => File.binread(File.join(support_dir, filename)), :headers => {"Content-Type" => "archive/zip"})
+end
+
 RSpec::Matchers.define :be_an_existing_file do
   match { |path| File.exist?(path) }
 end