Merge pull request #5572 from jekyll/fix-underscore-in-permalink

Merge pull request 5572

lib/jekyll/url.rb

@@ -84,17 +84,36 @@ module Jekyll
       end
     end
 
+    # We include underscores in keys to allow for 'i_month' and so forth.
+    # This poses a problem for keys which are followed by an underscore
+    # but the underscore is not part of the key, e.g. '/:month_:day'.
+    # That should be :month and :day, but our key extraction regexp isn't
+    # smart enough to know that so we have to make it an explicit
+    # possibility.
+    def possible_keys(key)
+      if key.end_with?("_")
+        [key, key.chomp("_")]
+      else
+        [key]
+      end
+    end
+
     def generate_url_from_drop(template)
       template.gsub(%r!:([a-z_]+)!) do |match|
-        key = match.sub(":".freeze, "".freeze)
-        unless @placeholders.key?(key)
-          raise NoMethodError, "The URL template key #{key} doesn't exist!"
-        end
-        if @placeholders[key].nil?
-          "".freeze
-        else
-          self.class.escape_path(@placeholders[key])
+        pool = possible_keys(match.sub(":".freeze, "".freeze))
+
+        winner = pool.find { |key| @placeholders.key?(key) }
+        if winner.nil?
+          raise NoMethodError,
+            "The URL template doesn't have #{pool.join(" or ")} keys. "\
+              "Check your permalink template!"
         end
+
+        value = @placeholders[winner]
+        value = "" if value.nil?
+        replacement = self.class.escape_path(value)
+
+        match.sub(":#{winner}", replacement)
       end.gsub(%r!//!, "/".freeze)
     end
 

test/helper.rb

@@ -103,6 +103,21 @@ class JekyllUnitTest < Minitest::Test
     RSpec::Mocks.teardown
   end
 
+  def fixture_document(relative_path)
+    site = fixture_site({
+      "collections" => {
+        "methods" => {
+          "output" => true
+        }
+      }
+    })
+    site.read
+    matching_doc = site.collections["methods"].docs.find do |doc|
+      doc.relative_path == relative_path
+    end
+    [site, matching_doc]
+  end
+
   def fixture_site(overrides = {})
     Jekyll::Site.new(site_configuration(overrides))
   end

test/test_url.rb

@@ -54,17 +54,7 @@ class TestURL < JekyllUnitTest
     end
 
     should "handle UrlDrop as a placeholder in addition to a hash" do
-      site = fixture_site({
-        "collections" => {
-          "methods" => {
-            "output" => true
-          }
-        }
-      })
-      site.read
-      matching_doc = site.collections["methods"].docs.find do |doc|
-        doc.relative_path == "_methods/escape-+ #%20[].md"
-      end
+      _, matching_doc = fixture_document("_methods/escape-+ #%20[].md")
       assert_equal "/methods/escape-+-20/escape-20.html", URL.new(
         :template     => "/methods/:title/:name:output_ext",
         :placeholders => matching_doc.url_placeholders
@@ -81,18 +71,16 @@ class TestURL < JekyllUnitTest
       end
     end
 
-    should "ignore NoMethodErrors when a placeholder is not found" do
-      site = fixture_site({
-        "collections" => {
-          "methods" => {
-            "output" => true
-          }
-        }
-      })
-      site.read
-      matching_doc = site.collections["methods"].docs.find do |doc|
-        doc.relative_path == "_methods/escape-+ #%20[].md"
-      end
+    should "check for key without trailing underscore" do
+      _, matching_doc = fixture_document("_methods/configuration.md")
+      assert_equal "/methods/configuration-configuration_methods_configuration", URL.new(
+        :template     => "/methods/:name-:slug_:collection_:title",
+        :placeholders => matching_doc.url_placeholders
+      ).to_s
+    end
+
+    should "raise custom error when URL placeholder doesn't have key" do
+      _, matching_doc = fixture_document("_methods/escape-+ #%20[].md")
       assert_raises NoMethodError do
         URL.new(
           :template     => "/methods/:headline",