Merge pull request #4403 from jekyll/page.dir-must-have-slash

Merge pull request 4403

benchmark/end-with-vs-regexp

@@ -4,10 +4,12 @@ Benchmark.ips do |x|
   path_without_ending_slash = '/some/very/very/long/path/to/a/file/i/like'
   x.report('no slash regexp')    { path_without_ending_slash =~ /\/$/ }
   x.report('no slash end_with?') { path_without_ending_slash.end_with?("/")  }
+  x.report('no slash [-1, 1]') { path_without_ending_slash[-1, 1] == "/"  }
 end
 
 Benchmark.ips do |x|
   path_with_ending_slash = '/some/very/very/long/path/to/a/file/i/like/'
   x.report('slash regexp')    { path_with_ending_slash =~ /\/$/ }
   x.report('slash end_with?') { path_with_ending_slash.end_with?("/")  }
+  x.report('slash [-1, 1]')   { path_with_ending_slash[-1, 1] == "/"  }
 end

benchmark/file-dir-ensure-trailing-slash

@@ -0,0 +1,54 @@
+#!/usr/bin/env ruby
+require 'benchmark/ips'
+
+# For this pull request, which changes Page#dir
+# https://github.com/jekyll/jekyll/pull/4403
+
+FORWARD_SLASH = '/'.freeze
+
+def pre_pr(url)
+  url[-1, 1] == FORWARD_SLASH ? url : File.dirname(url)
+end
+
+def pr(url)
+  if url.end_with?(FORWARD_SLASH)
+    url
+  else
+    url_dir = File.dirname(url)
+    url_dir.end_with?(FORWARD_SLASH) ? url_dir : "#{url_dir}/"
+  end
+end
+
+def envygeeks(url)
+  return url if url.end_with?(FORWARD_SLASH) || url == FORWARD_SLASH
+
+  url = File.dirname(url)
+  url == FORWARD_SLASH ? url : "#{url}/"
+end
+
+# Just a slash
+Benchmark.ips do |x|
+  path = '/'
+  x.report("pre_pr:#{path}")    { pre_pr(path) }
+  x.report("pr:#{path}")        { pr(path) }
+  x.report("envygeeks:#{path}") { pr(path) }
+  x.compare!
+end
+
+# No trailing slash
+Benchmark.ips do |x|
+  path = '/some/very/very/long/path/to/a/file/i/like'
+  x.report("pre_pr:#{path}")    { pre_pr(path) }
+  x.report("pr:#{path}")        { pr(path) }
+  x.report("envygeeks:#{path}") { pr(path) }
+  x.compare!
+end
+
+# No trailing slash
+Benchmark.ips do |x|
+  path = '/some/very/very/long/path/to/a/file/i/like/'
+  x.report("pre_pr:#{path}")    { pre_pr(path) }
+  x.report("pr:#{path}")        { pr(path) }
+  x.report("envygeeks:#{path}") { pr(path) }
+  x.compare!
+end

lib/jekyll/page.rb

@@ -9,6 +9,8 @@ module Jekyll
 
     alias_method :extname, :ext
 
+    FORWARD_SLASH = '/'.freeze
+
     # Attributes for Liquid templates
     ATTRIBUTES_FOR_LIQUID = %w(
       content
@@ -55,7 +57,12 @@ module Jekyll
     #
     # Returns the String destination directory.
     def dir
-      url[-1, 1] == '/' ? url : File.dirname(url)
+      if url.end_with?(FORWARD_SLASH)
+        url
+      else
+        url_dir = File.dirname(url)
+        url_dir.end_with?(FORWARD_SLASH) ? url_dir : "#{url_dir}/"
+      end
     end
 
     # The full path and filename of the post. Defined in the YAML of the post

test/test_page.rb

@@ -150,6 +150,12 @@ class TestPage < JekyllUnitTest
           assert_equal '/contacts.html', @page.url
           assert_equal @dest_file, @page.destination(dest_dir)
         end
+
+        should "return dir correctly" do
+          assert_equal '/', setup_page('contacts.html').dir
+          assert_equal '/contacts/', setup_page('contacts/bar.html').dir
+          assert_equal '/contacts/', setup_page('contacts/index.html').dir
+        end
       end
 
       context "with custom permalink style with trailing slash" do
@@ -194,8 +200,9 @@ class TestPage < JekyllUnitTest
       context "with any other permalink style" do
         should "return dir correctly" do
           @site.permalink_style = nil
-          @page = setup_page('contacts.html')
-          assert_equal '/', @page.dir
+          assert_equal '/', setup_page('contacts.html').dir
+          assert_equal '/contacts/', setup_page('contacts/index.html').dir
+          assert_equal '/contacts/', setup_page('contacts/bar.html').dir
         end
       end