From ce3fa7fd0239f8eceafe21f17e9da9074f1904da Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Sat, 8 Jun 2013 19:49:39 +0200 Subject: [PATCH 1/8] Add feature to test new functionality. --- features/pagination.feature | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/features/pagination.feature b/features/pagination.feature index 4259caf1..76c62361 100644 --- a/features/pagination.feature +++ b/features/pagination.feature @@ -52,3 +52,31 @@ Feature: Site pagination | 2 | 1 | 5 | | 3 | 1 | 6 | | 4 | 1 | 7 | + + Scenario Outline: Setting a custom pagination path without an index.html in it + Given I have a configuration file with: + | key | value | + | paginate | 1 | + | paginate_path | /blog/page/:num | + | permalink | /blog/:year/:month/:day/:title | + And I have a blog directory + And I have an "blog/index.html" page that contains "{{ paginator.posts.size }}" + And I have an "index.html" page that contains "Don't pick me!" + And I have a _posts directory + And I have the following posts: + | title | date | layout | content | + | Wargames | 2009-03-27 | default | The only winning move is not to play. | + | Wargames2 | 2009-04-27 | default | The only winning move is not to play2. | + | Wargames3 | 2009-05-27 | default | The only winning move is not to play3. | + | Wargames4 | 2009-06-27 | default | The only winning move is not to play4. | + When I run jekyll + Then the _site/blog/page/ directory should exist + And the "_site/blog/page//index.html" file should exist + And I should see "" in "_site/blog/page//index.html" + And the "_site/blog/page//index.html" file should not exist + + Examples: + | exist | posts | not_exist | + | 2 | 1 | 5 | + | 3 | 1 | 6 | + | 4 | 1 | 7 | From a71b755e9e539d595ba47c27832978f2f4b3fd09 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Sat, 8 Jun 2013 19:49:58 +0200 Subject: [PATCH 2/8] Remove superfluous "After" block from step defintions. --- features/step_definitions/jekyll_steps.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/features/step_definitions/jekyll_steps.rb b/features/step_definitions/jekyll_steps.rb index 7a1c7cd1..626a58a5 100644 --- a/features/step_definitions/jekyll_steps.rb +++ b/features/step_definitions/jekyll_steps.rb @@ -4,11 +4,6 @@ Before do Dir.chdir(TEST_DIR) end -After do - Dir.chdir(File.expand_path("..", TEST_DIR)) - FileUtils.rm_rf(TEST_DIR) -end - Given /^I have a blank site in "(.*)"$/ do |path| FileUtils.mkdir(path) end From bd0e45c1b9b3bd8c51ddc64d5909c2c413dab165 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Sat, 8 Jun 2013 20:23:44 +0200 Subject: [PATCH 3/8] Fixed bugs with pagination in a subdirectory. --- lib/jekyll/generators/pagination.rb | 105 +++++++++++++++++++++------- 1 file changed, 80 insertions(+), 25 deletions(-) diff --git a/lib/jekyll/generators/pagination.rb b/lib/jekyll/generators/pagination.rb index cfcd9c30..cd90a2c1 100644 --- a/lib/jekyll/generators/pagination.rb +++ b/lib/jekyll/generators/pagination.rb @@ -10,9 +10,7 @@ module Jekyll # # Returns nothing. def generate(site) - site.pages.dup.each do |page| - paginate(site, page) if Pager.pagination_enabled?(site.config, page) - end + paginate(site, template_page(site)) if Pager.pagination_enabled?(site) end # Paginates the blog's posts. Renders the index.html file into paginated @@ -33,11 +31,11 @@ module Jekyll all_posts = site.site_payload['site']['posts'] pages = Pager.calculate_pages(all_posts, site.config['paginate'].to_i) (1..pages).each do |num_page| - pager = Pager.new(site.config, num_page, all_posts, pages) + pager = Pager.new(site, num_page, all_posts, pages) if num_page > 1 newpage = Page.new(site, site.source, page.dir, page.name) newpage.pager = pager - newpage.dir = File.join(page.dir, Pager.paginate_path(site.config, num_page)) + newpage.dir = Pager.paginate_path(site, num_page) site.pages << newpage else page.pager = pager @@ -45,6 +43,28 @@ module Jekyll end end + # Static: Fetch the URL of the template page. Used to determine the + # path to the first pager in the series. + # + # site - the Jekyll::Site object + # + # Returns the url of the template page + def self.first_page_url(site) + Pagination.new.template_page(site).url + end + + # Public: Find the Jekyll::Page which will act as the pager template + # + # site - the Jekyll::Site object + # + # Returns the Jekyll::Page which will act as the pager template + def template_page(site) + site.pages.dup.select do |page| + Pager.pagination_candidate?(site.config, page) + end.sort do |one, two| + two.path.size <=> one.path.size + end.first + end end end @@ -62,39 +82,74 @@ module Jekyll (all_posts.size.to_f / per_page.to_i).ceil end - # Determine if pagination is enabled for a given file. + # Determine if pagination is enabled the site. # - # config - The configuration Hash. - # page - The Jekyll::Page with which to paginate + # site - the Jekyll::Site object # # Returns true if pagination is enabled, false otherwise. - def self.pagination_enabled?(config, page) - !config['paginate'].nil? && - page.name == 'index.html' && - subdirectories_identical(config['paginate_path'], page.dir) + def self.pagination_enabled?(site) + !site.config['paginate'].nil? && + site.pages.size > 0 + end + + # Static: Determine if a page is a possible candidate to be a template page. + # Page's name must be `index.html` and exist in any of the directories + # between the site source and `paginate_path`. + # + # Returns true if the + def self.pagination_candidate?(config, page) + page_dir = File.dirname(File.expand_path(remove_leading_slash(page.path), config['source'])) + page.name == 'index.html' && + in_hierarchy(config, page_dir) end # Determine if the subdirectories of the two paths are the same relative to source # - # paginate_path - the paginate_path configuration setting - # page_dir - the directory of the Jekyll::Page + # config - the site configuration hash + # page_dir - the directory of the Jekyll::Page # # Returns whether the subdirectories are the same relative to source - def self.subdirectories_identical(paginate_path, page_dir) - File.dirname(paginate_path).gsub(/\A\./, '') == page_dir.gsub(/\/\z/, '') + def self.in_hierarchy(config, page_dir) + paginate_path = remove_leading_slash(config['paginate_path']) + paginate_path = File.expand_path(paginate_path, config['source']) + return false if page_dir == File.dirname(page_dir) + page_dir == config['source'] || + page_dir == paginate_path || + in_hierarchy(config, File.dirname(page_dir)) end # Static: Return the pagination path of the page # - # site_config - the site config + # site - the Jekyll::Site object # num_page - the pagination page number # # Returns the pagination path as a string - def self.paginate_path(site_config, num_page) - return nil if num_page.nil? || num_page <= 1 - format = site_config['paginate_path'] + def self.paginate_path(site, num_page) + return nil if num_page.nil? + return Generators::Pagination.first_page_url(site) if num_page <= 1 + format = site.config['paginate_path'] format = format.sub(':num', num_page.to_s) - File.basename(format) + ensure_leading_slash(format) + end + + # Static: Return a String version of the input which has a leading slash. + # If the input already has a forward slash in position zero, it will be + # returned unchanged. + # + # path - a String path + # + # Returns the path with a leading slash + def self.ensure_leading_slash(path) + path[0..0] == "/" ? path : "/#{path}" + end + + # Static: Return a String version of the input without a leading slash. + # + # path - a String path + # + # Returns the input without the leading slash + def self.remove_leading_slash(path) + ensure_leading_slash(path)[1..-1] end # Initialize a new Pager. @@ -104,9 +159,9 @@ module Jekyll # all_posts - The Array of all the site's Posts. # num_pages - The Integer number of pages or nil if you'd like the number # of pages calculated. - def initialize(config, page, all_posts, num_pages = nil) + def initialize(site, page, all_posts, num_pages = nil) @page = page - @per_page = config['paginate'].to_i + @per_page = site.config['paginate'].to_i @total_pages = num_pages || Pager.calculate_pages(all_posts, @per_page) if @page > @total_pages @@ -119,9 +174,9 @@ module Jekyll @total_posts = all_posts.size @posts = all_posts[init..offset] @previous_page = @page != 1 ? @page - 1 : nil - @previous_page_path = Pager.paginate_path(config, @previous_page) + @previous_page_path = Pager.paginate_path(site, @previous_page) @next_page = @page != @total_pages ? @page + 1 : nil - @next_page_path = Pager.paginate_path(config, @next_page) + @next_page_path = Pager.paginate_path(site, @next_page) end # Convert this Pager's data to a Hash suitable for use by Liquid. From 81a2d03e58f1a5b5404177c4cd848d808f9696b5 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Sun, 9 Jun 2013 18:06:28 +0200 Subject: [PATCH 4/8] =?UTF-8?q?WIP=20=E2=80=93=20fixing=20tests.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/jekyll/generators/pagination.rb | 6 +++++- test/test_pager.rb | 19 +++++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/jekyll/generators/pagination.rb b/lib/jekyll/generators/pagination.rb index cd90a2c1..6599aa4c 100644 --- a/lib/jekyll/generators/pagination.rb +++ b/lib/jekyll/generators/pagination.rb @@ -50,7 +50,11 @@ module Jekyll # # Returns the url of the template page def self.first_page_url(site) - Pagination.new.template_page(site).url + if page = Pagination.new.template_page(site) + page.url + else + nil + end end # Public: Find the Jekyll::Page which will act as the pager template diff --git a/test/test_pager.rb b/test/test_pager.rb index 990d28eb..b4a22c9b 100644 --- a/test/test_pager.rb +++ b/test/test_pager.rb @@ -2,6 +2,15 @@ require 'helper' class TestPager < Test::Unit::TestCase + def jekyll_site(config = {}) + base = Jekyll::Configuration::DEFAULTS.merge({ + 'source' => source_dir, + 'destination' => dest_dir, + 'paginate' => 1 + }) + Jekyll::Site.new(base.merge(config)) + end + should "calculate number of pages" do assert_equal(0, Pager.calculate_pages([], '2')) assert_equal(1, Pager.calculate_pages([1], '2')) @@ -12,10 +21,12 @@ class TestPager < Test::Unit::TestCase end should "determine the pagination path" do - assert_nil(Pager.paginate_path(Jekyll::Configuration::DEFAULTS, 1)) - assert_equal("page2", Pager.paginate_path(Jekyll::Configuration::DEFAULTS, 2)) - assert_nil(Pager.paginate_path(Jekyll::Configuration::DEFAULTS.merge('paginate_path' => '/blog/page-:num'), 1)) - assert_equal("page-2", Pager.paginate_path(Jekyll::Configuration::DEFAULTS.merge('paginate_path' => '/blog/page-:num'), 2)) + assert_equal("/index.html", Pager.paginate_path(jekyll_site, 1)) + assert_equal("/page2", Pager.paginate_path(jekyll_site, 2)) + assert_equal("/index.html", Pager.paginate_path(jekyll_site('paginate_path' => '/blog/page-:num'), 1)) + assert_equal("/blog/page-2", Pager.paginate_path(jekyll_site('paginate_path' => '/blog/page-:num'), 2)) + assert_equal("/index.html", Pager.paginate_path(jekyll_site('paginate_path' => '/blog/page/:num'), 1)) + assert_equal("/blog/page/2", Pager.paginate_path(jekyll_site('paginate_path' => '/blog/page/:num'), 2)) end context "pagination disabled" do From 6e80ba868e9c7b0546d97a1a9b9766f0f5c1a14a Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Sun, 9 Jun 2013 18:52:24 +0200 Subject: [PATCH 5/8] Fix tests for Pager. --- lib/jekyll/configuration.rb | 2 +- test/test_pager.rb | 65 +++++++++++++------------------------ 2 files changed, 23 insertions(+), 44 deletions(-) diff --git a/lib/jekyll/configuration.rb b/lib/jekyll/configuration.rb index 1a915731..4a1f555f 100644 --- a/lib/jekyll/configuration.rb +++ b/lib/jekyll/configuration.rb @@ -29,7 +29,7 @@ module Jekyll 'baseurl' => '/', 'include' => ['.htaccess'], 'exclude' => [], - 'paginate_path' => 'page:num', + 'paginate_path' => '/page:num', 'markdown_ext' => 'markdown,mkd,mkdn,md', 'textile_ext' => 'textile', diff --git a/test/test_pager.rb b/test/test_pager.rb index b4a22c9b..f81f57f6 100644 --- a/test/test_pager.rb +++ b/test/test_pager.rb @@ -2,13 +2,15 @@ require 'helper' class TestPager < Test::Unit::TestCase - def jekyll_site(config = {}) - base = Jekyll::Configuration::DEFAULTS.merge({ + def build_site(config = {}) + base = Jekyll::Configuration::DEFAULTS.deep_merge({ 'source' => source_dir, 'destination' => dest_dir, - 'paginate' => 1 + 'pagination' => 1 }) - Jekyll::Site.new(base.merge(config)) + site = Jekyll::Site.new(base.deep_merge(config)) + site.process + site end should "calculate number of pages" do @@ -21,51 +23,28 @@ class TestPager < Test::Unit::TestCase end should "determine the pagination path" do - assert_equal("/index.html", Pager.paginate_path(jekyll_site, 1)) - assert_equal("/page2", Pager.paginate_path(jekyll_site, 2)) - assert_equal("/index.html", Pager.paginate_path(jekyll_site('paginate_path' => '/blog/page-:num'), 1)) - assert_equal("/blog/page-2", Pager.paginate_path(jekyll_site('paginate_path' => '/blog/page-:num'), 2)) - assert_equal("/index.html", Pager.paginate_path(jekyll_site('paginate_path' => '/blog/page/:num'), 1)) - assert_equal("/blog/page/2", Pager.paginate_path(jekyll_site('paginate_path' => '/blog/page/:num'), 2)) + #assert_equal("/index.html", Pager.paginate_path(build_site, 1)) + assert_equal("/page2", Pager.paginate_path(build_site, 2)) + #assert_equal("/index.html", Pager.paginate_path(build_site('paginate_path' => '/blog/page-:num'), 1)) + assert_equal("/blog/page-2", Pager.paginate_path(build_site('paginate_path' => '/blog/page-:num'), 2)) + #assert_equal("/index.html", Pager.paginate_path(build_site('paginate_path' => '/blog/page/:num'), 1)) + assert_equal("/blog/page/2", Pager.paginate_path(build_site('paginate_path' => '/blog/page/:num'), 2)) end context "pagination disabled" do - setup do - stub(Jekyll).configuration do - Jekyll::Configuration::DEFAULTS.merge({ - 'source' => source_dir, - 'destination' => dest_dir - }) - end - @config = Jekyll.configuration - end - should "report that pagination is disabled" do - page = OpenStruct.new({ :name => 'index.html', :dir => '/' }) - assert !Pager.pagination_enabled?(@config, page) + assert !Pager.pagination_enabled?(build_site('paginate' => nil)) end - end context "pagination enabled for 2" do setup do - stub(Jekyll).configuration do - Jekyll::Configuration::DEFAULTS.merge({ - 'source' => source_dir, - 'destination' => dest_dir, - 'paginate' => 2 - }) - end - - @config = Jekyll.configuration - @site = Site.new(@config) - @site.process + @site = build_site('paginate' => 2) @posts = @site.posts end should "report that pagination is enabled" do - page = OpenStruct.new({ :name => 'index.html', :dir => '/' }) - assert Pager.pagination_enabled?(@config, page) + assert Pager.pagination_enabled?(@site) end context "with 4 posts" do @@ -74,7 +53,7 @@ class TestPager < Test::Unit::TestCase end should "create first pager" do - pager = Pager.new(@config, 1, @posts) + pager = Pager.new(@site, 1, @posts) assert_equal(2, pager.posts.size) assert_equal(2, pager.total_pages) assert_nil(pager.previous_page) @@ -82,7 +61,7 @@ class TestPager < Test::Unit::TestCase end should "create second pager" do - pager = Pager.new(@config, 2, @posts) + pager = Pager.new(@site, 2, @posts) assert_equal(2, pager.posts.size) assert_equal(2, pager.total_pages) assert_equal(1, pager.previous_page) @@ -90,7 +69,7 @@ class TestPager < Test::Unit::TestCase end should "not create third pager" do - assert_raise(RuntimeError) { Pager.new(@config, 3, @posts) } + assert_raise(RuntimeError) { Pager.new(@site, 3, @posts) } end end @@ -101,7 +80,7 @@ class TestPager < Test::Unit::TestCase end should "create first pager" do - pager = Pager.new(@config, 1, @posts) + pager = Pager.new(@site, 1, @posts) assert_equal(2, pager.posts.size) assert_equal(3, pager.total_pages) assert_nil(pager.previous_page) @@ -109,7 +88,7 @@ class TestPager < Test::Unit::TestCase end should "create second pager" do - pager = Pager.new(@config, 2, @posts) + pager = Pager.new(@site, 2, @posts) assert_equal(2, pager.posts.size) assert_equal(3, pager.total_pages) assert_equal(1, pager.previous_page) @@ -117,7 +96,7 @@ class TestPager < Test::Unit::TestCase end should "create third pager" do - pager = Pager.new(@config, 3, @posts) + pager = Pager.new(@site, 3, @posts) assert_equal(1, pager.posts.size) assert_equal(3, pager.total_pages) assert_equal(2, pager.previous_page) @@ -125,7 +104,7 @@ class TestPager < Test::Unit::TestCase end should "not create fourth pager" do - assert_raise(RuntimeError) { Pager.new(@config, 4, @posts) } + assert_raise(RuntimeError) { Pager.new(@site, 4, @posts) } end end From 05218711b2c9026dfe4849d164a489afe18b205d Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Sun, 9 Jun 2013 21:08:29 +0200 Subject: [PATCH 6/8] Add warning when pagination is enabled but no template page has been found --- lib/jekyll/generators/pagination.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/jekyll/generators/pagination.rb b/lib/jekyll/generators/pagination.rb index 6599aa4c..daded7b1 100644 --- a/lib/jekyll/generators/pagination.rb +++ b/lib/jekyll/generators/pagination.rb @@ -10,7 +10,14 @@ module Jekyll # # Returns nothing. def generate(site) - paginate(site, template_page(site)) if Pager.pagination_enabled?(site) + if Pager.pagination_enabled?(site) + if template = template_page(site) + paginate(site, template) + else + Jekyll.logger.warn "Pagination:", "Pagination is enabled, but I couldn't find" + + "an index.html page to use as the pagination template. Skipping pagination." + end + end end # Paginates the blog's posts. Renders the index.html file into paginated From 7dc6767bc9a3f6d9ede5160f3f387e1f93da56b9 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Sun, 9 Jun 2013 21:08:45 +0200 Subject: [PATCH 7/8] Fix Pager.in_hierarchy to actually do the right thing --- lib/jekyll/generators/pagination.rb | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/jekyll/generators/pagination.rb b/lib/jekyll/generators/pagination.rb index daded7b1..a9c279a1 100644 --- a/lib/jekyll/generators/pagination.rb +++ b/lib/jekyll/generators/pagination.rb @@ -107,26 +107,30 @@ module Jekyll # Page's name must be `index.html` and exist in any of the directories # between the site source and `paginate_path`. # + # config - the site configuration hash + # page - the Jekyll::Page about which we're inquiring + # # Returns true if the def self.pagination_candidate?(config, page) page_dir = File.dirname(File.expand_path(remove_leading_slash(page.path), config['source'])) + paginate_path = remove_leading_slash(config['paginate_path']) + paginate_path = File.expand_path(paginate_path, config['source']) page.name == 'index.html' && - in_hierarchy(config, page_dir) + in_hierarchy(config['source'], page_dir, File.dirname(paginate_path)) end # Determine if the subdirectories of the two paths are the same relative to source # - # config - the site configuration hash - # page_dir - the directory of the Jekyll::Page + # source - the site source + # page_dir - the directory of the Jekyll::Page + # paginate_path - the absolute paginate path (from root of FS) # # Returns whether the subdirectories are the same relative to source - def self.in_hierarchy(config, page_dir) - paginate_path = remove_leading_slash(config['paginate_path']) - paginate_path = File.expand_path(paginate_path, config['source']) - return false if page_dir == File.dirname(page_dir) - page_dir == config['source'] || - page_dir == paginate_path || - in_hierarchy(config, File.dirname(page_dir)) + def self.in_hierarchy(source, page_dir, paginate_path) + return false if paginate_path == File.dirname(paginate_path) + return false if paginate_path == Pathname.new(source).parent + page_dir == paginate_path || + in_hierarchy(source, page_dir, File.dirname(paginate_path)) end # Static: Return the pagination path of the page From 7f10b324315801c457ba41de5f6b669426789856 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Sun, 9 Jun 2013 21:09:06 +0200 Subject: [PATCH 8/8] Fix tests to match new behaviour of Pager.paginate_path --- test/test_pager.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/test/test_pager.rb b/test/test_pager.rb index f81f57f6..b066b9a3 100644 --- a/test/test_pager.rb +++ b/test/test_pager.rb @@ -6,7 +6,7 @@ class TestPager < Test::Unit::TestCase base = Jekyll::Configuration::DEFAULTS.deep_merge({ 'source' => source_dir, 'destination' => dest_dir, - 'pagination' => 1 + 'paginate' => 1 }) site = Jekyll::Site.new(base.deep_merge(config)) site.process @@ -23,12 +23,16 @@ class TestPager < Test::Unit::TestCase end should "determine the pagination path" do - #assert_equal("/index.html", Pager.paginate_path(build_site, 1)) + assert_equal("/index.html", Pager.paginate_path(build_site, 1)) assert_equal("/page2", Pager.paginate_path(build_site, 2)) - #assert_equal("/index.html", Pager.paginate_path(build_site('paginate_path' => '/blog/page-:num'), 1)) - assert_equal("/blog/page-2", Pager.paginate_path(build_site('paginate_path' => '/blog/page-:num'), 2)) - #assert_equal("/index.html", Pager.paginate_path(build_site('paginate_path' => '/blog/page/:num'), 1)) - assert_equal("/blog/page/2", Pager.paginate_path(build_site('paginate_path' => '/blog/page/:num'), 2)) + assert_equal("/index.html", Pager.paginate_path(build_site({'paginate_path' => '/blog/page-:num'}), 1)) + assert_equal("/blog/page-2", Pager.paginate_path(build_site({'paginate_path' => '/blog/page-:num'}), 2)) + assert_equal("/index.html", Pager.paginate_path(build_site({'paginate_path' => '/blog/page/:num'}), 1)) + assert_equal("/blog/page/2", Pager.paginate_path(build_site({'paginate_path' => '/blog/page/:num'}), 2)) + assert_equal("/contacts/index.html", Pager.paginate_path(build_site({'paginate_path' => '/contacts/page:num'}), 1)) + assert_equal("/contacts/page2", Pager.paginate_path(build_site({'paginate_path' => '/contacts/page:num'}), 2)) + assert_equal("/contacts/index.html", Pager.paginate_path(build_site({'paginate_path' => '/contacts/page/:num'}), 1)) + assert_equal("/contacts/page/2", Pager.paginate_path(build_site({'paginate_path' => '/contacts/page/:num'}), 2)) end context "pagination disabled" do