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

Refactor tags and categories #1639

Merged
merged 1 commit into from Mar 31, 2014

Conversation

Projects
None yet
5 participants
@maul-esel
Contributor

maul-esel commented Oct 17, 2013

Until now, Site maintained a list of tags and categories where each tag or category as associated with the posts that had the tag / belonged to the category. At the same time, Site#post_attr_hash produces such a hash.

So to avoid the duplication, this PR removes the special handling of tags and categories in favor of the more generic implementation in #post_attr_hash. In case any plugins use Site#tags and Site#categories (it's never used inside jekyll, except for the tests), I added dummy methods for them that just call #post_attr_hash.

@kelvinst

This comment has been minimized.

kelvinst commented Oct 17, 2013

This is related to #670

But, since this doesn't hurt the actual behavior, I think it can be merged before that old PR.

@maul-esel

This comment has been minimized.

Contributor

maul-esel commented Oct 17, 2013

The travis failure is that old time issue again, not related to this PR. It actually builds fine.

@parkr parkr closed this Mar 17, 2014

@parkr parkr reopened this Mar 17, 2014

@parkr

This comment has been minimized.

Member

parkr commented Mar 30, 2014

Hey, would you be willing to rebase this?

remove duplicate code for tags and categories
Previously the `Site#tags` and `Site#categories` actually had the
same structure as a hash returned by `Site#post_attr_hash()`, but
maintained separately. Remove this duplicated infrastructure. For
backwards-compatibility for plugins, provide aliases for them.
@maul-esel

This comment has been minimized.

Contributor

maul-esel commented Mar 30, 2014

No problem.

@parkr

This comment has been minimized.

Member

parkr commented Mar 31, 2014

You rock, thanks @maul-esel.

@mattr- Thoughts on this?

@mattr-

This comment has been minimized.

Member

mattr- commented Mar 31, 2014

👍

@@ -277,10 +272,18 @@ def post_attr_hash(post_attr)
# array of posts ) then sort each array in reverse order.
hash = Hash.new { |h, key| h[key] = [] }
posts.each { |p| p.send(post_attr.to_sym).each { |t| hash[t] << p } }
hash.values.map { |sortme| sortme.sort! { |a, b| b <=> a } }
hash.values.each { |posts| posts.sort!.reverse! }

This comment has been minimized.

@parkr

parkr Mar 31, 2014

Member

Are posts sorted in the "correct" order still? Not sure if that's tested. It looks from the old code like it needs to be in reverse-chronological order.

@parkr parkr merged commit bc7aed2 into jekyll:master Mar 31, 2014

1 check passed

default The Travis CI build passed
Details

parkr added a commit that referenced this pull request Mar 31, 2014

@parkr parkr added this to the 2.0 milestone Mar 31, 2014

@maul-esel maul-esel deleted the maul-esel:tags-and-categories branch Mar 31, 2014

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.