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

(WIP) Jekyll3 Support #7

Closed
wants to merge 1 commit into from
Closed

(WIP) Jekyll3 Support #7

wants to merge 1 commit into from

Conversation

imathis
Copy link
Member

@imathis imathis commented Jul 11, 2015

Add support for native Jekyll hooks.

@imathis
Copy link
Member Author

imathis commented Jul 11, 2015

I've tracked down my problems to the fact that apparently my Jekyll::Page subclass doesn't trigger Jekyll hooks. You can find that subclass here

@parkr, have any ideas what I can do to get this joker to trigger the :pre_render page hook?

@imathis imathis mentioned this pull request Jul 11, 2015
@parkr
Copy link
Member

parkr commented Jul 27, 2015

@imathis Thanks for working on this! I think the problem with the hooks is that we don't have one for :page, :post_render afaict. @envygeeks, is this intentional? What's the alternative?

@paulrobertlloyd
Copy link

Trying out this branch, as I’m also using Jekyll 3.0.beta (I like to live dangerously). Right now this plugin generates the right number of pages for a paginated template, but pages beyond the first don’t contain any posts. Does this issue relate to what I’m seeing?

For reference, here’s my template:

---
layout: index
title: Journal
paginate:
  permalink: :num/
  limit: false
  per_page: 12
  title_suffix: ''
---
{% for item in paginator.posts %}
    {% case item.layout %}
    {% when "post-link" %}
        {% include item/link.html %}
    {% when "post-album" %}
        {% include item/album.html %}
    {% else %}
        {% include item/entry.html %}
    {% endcase %}
{% endfor %}

(After porting my existing pagination over to this plugin, hoping to see how far it will allow me to paginate other collections, something that I’ve been hoping to see in jekyll-paginate for a while now. Secretly hoping it will allow me to paginate tag pages, too!)

@envygeeks
Copy link

@parkr @imathis this was originally intentional to make hooks easier, but I've been discussing with @stevecrozz making hooks more flexible so that hooks can be triggered with data without the need of it relating to a class, this way our ability to add hooks scales better.

@stevecrozz
Copy link

@parkr Why do you say we don't have a page post_render hook? There's a cheesy example here in the cucumber tests: https://github.com/jekyll/jekyll/blob/v3.0.0.pre.beta8/features/hooks.feature#L101-L111

@envygeeks
Copy link

@stevecrozz we do but @imathis hook isn't triggering because the #class doesn't match, whereas #is_a? would match because it tracks sub-classes.

@envygeeks
Copy link

I would personally like to see the reliance on classes removed in favor of triggering a point and a hook set so that things are more flexible to both plugins and us (re: the configuration hook problem we discussed a week ago.)

@stevecrozz
Copy link

Ah. He has a subclass of Page and not an instance of Page. Thanks @envygeeks.

I opened an issue with jekyll to track this: jekyll/jekyll#3870 and I suppose we can discuss it over there.

@mshick
Copy link

mshick commented Nov 18, 2015

This PR and the jekyll3 branch is working well for me on jekyll 3.0.1. Maybe there were incompatibilities in the pre-release versions that have been dealt with?

I'd imagine there are improvements that can be made to the codebase, but would love to have the released gem working with jekyll 3.

Edit: There is a small bug in the PR however -- posted the fix inline.


class PageHook < Hooks::Page
def merge_payload(payload, page)
Jekyll::Hooks.register :page, :pre_render do |page, payload|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be :pages not :page

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mshick technically that's a yes, but soon it's a no, we plan on having aliases and this will be one of them, unless @parkr changed his mind with my plan to add hook aliases to ease developer pain with hooks, his hooks will continue to work soon.

@ChrisChinchilla
Copy link

@imathis @mshick This plugin not working with Jekyll 3 is currently breaking my upgrade, not sure if this is helpful, but on build / serve I get this error repeatedly:

"Deprecation: Document#categories is now a key in the #data hash.
Called by /usr/local/lib/ruby/gems/2.3.0/bundler/gems/paginate-f15799044ad6/lib/octopress-paginate.rb:147:in `block in collection'."

The site build and serves, but anything using pagination is blank.

Happy to help fix if someone can point me in the right direction, lack of proper and decent pagination in default Jekyll makes this plugin invaluable!

@mshick
Copy link

mshick commented Feb 1, 2016

@ChrisChinchilla this plugin is broken in Jekyll 3. My fork fixes all warnings and problems, and is based upon an earlier patch made by @imathis. I'm not sure what the hold-up is getting out a proper fix...

You can use my fork by adding this to your Gemfile and bundle installing

gem "octopress-paginate", :git => "git://github.com/mshick/paginate", :branch => "jekyll3"

@ChrisChinchilla
Copy link

@mshick Thanks! I was using the jekyll3 branch of this repo. I can confirm yours does fix everything. Awesome!

@timwis
Copy link

timwis commented Feb 18, 2016

Hey guys, what's going on with this PR? Looks like octopress-paginate doesn't work with Jekyll 3 still. Should I use the fork mentioned above? Has anyone submitted a PR from the fork to this repo?

@mshick
Copy link

mshick commented Feb 18, 2016

@timwis If you follow this convo you'll see there are some code style issues holding this up. It's a major rewrite, so I think the maintainer needs to set it off. Until then I can say my fork works quite well.

@lorenzschmid
Copy link

@mshick your fork works great, thank you! Why not merge the PR?

@mkava
Copy link

mkava commented May 12, 2016

@mshick Solid changes with your fork mate. No more deprecation messages is quite appreciated!

@parkr
Copy link
Member

parkr commented May 12, 2016

@mshick If you submit your fork for a PR, I can work out a release for this.

@parkr parkr closed this May 12, 2016
@parkr parkr deleted the jekyll3 branch May 12, 2016 15:09
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

Successfully merging this pull request may close these issues.

10 participants