Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

previous_page_path includes index.html #15

Open
rovrov opened this issue Nov 9, 2014 · 7 comments
Open

previous_page_path includes index.html #15

rovrov opened this issue Nov 9, 2014 · 7 comments

Comments

@rovrov
Copy link

rovrov commented Nov 9, 2014

Moved from jekyll/jekyll#3073

paginator.previous_page_path generates base/page2 when on the third page, but for the second page the result is base/index.html which is not pretty. Shouldn't previous_page_path generate just base/ in this case?

Here is a sample fix: rovrov@088fd18

base instead of base/ (i.e. chomp("/index.html")) might be even more consistent if it always works without the trailing slash.

@parkr
Copy link
Member

parkr commented Nov 9, 2014

My only concern here is that not everyone will want this.

@rovrov
Copy link
Author

rovrov commented Nov 9, 2014

The way I see it, the following options are consistent:

  • base/page2/ and base/
  • base/page2/index.html and base/index.html

I don't know why anyone would want to have a mix between the two.

Also, base/page2 without the trailing slash is not great because GH first responds 301 "Moved Permanently" --> base/page2/ and then an additional request to base/page2/ happens.

@parkr
Copy link
Member

parkr commented Nov 10, 2014

Yeah the consistency makes sense for sure.

The 301 may not be Jekyll. What permalink style do you have set in your config.yml? Maybe we need to default to /base/page:num/

@rovrov
Copy link
Author

rovrov commented Nov 10, 2014

I was using paginate_path: "blog/page:num" that I took from http://jekyllrb.com/docs/pagination/. It should indeed be "blog/page:num/", could you update the docs as it is misleading? Not sure about the defaults - doesn't work for me if I don't set a paginate_path.

There is nginx keyword in the 301 response-body, anyway 301 is the right thing in this case.

I just realized someone might have set paginate_path: "blog/page:num/index.html", and then "blog/index.html" would be appropriate for the first page. The question is if people actually do this.

@parkr
Copy link
Member

parkr commented Nov 10, 2014

It should indeed be "blog/page:num/"

Yeah, we can update the docs. Would you mind submitting a PR? The docs are in jekyll/jekyll in the master branch in the site/_docs/pagination.md file.

I just realized someone might have set paginate_path: "blog/page:num/index.html", and then "blog/index.html" would be appropriate for the first page. The question is if people actually do this.

I don't think anyone does this, but we can't be sure.

@rovrov
Copy link
Author

rovrov commented Nov 10, 2014

Submitted: jekyll/jekyll#3091

Concerning the base/index.html issue, just want to add that other people also had to fix it and blogged about this:
http://www.ericlagergren.com/blog/jekyll-pagination/
| replace: 'index.html', '/'

http://www.paulwrankin.com/blog/2014/10/16/better-jekyll-pagination/
{% if paginator.previous_page == 1 %}

@neoascetic
Copy link

This is fixed for Jekyll > 3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants