Skip to content
New issue

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

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

Already on GitHub? # to your account

Read-in site's tags and categories attributes #137

Merged
merged 1 commit into from
Apr 17, 2019

Conversation

ashmaroli
Copy link
Member

Jekyll::Site#tags and Jekyll::Site#categories (introduced in Jekyll 2.0 via jekyll/jekyll@8c0e5d8) already encapsulate the exact same logic:

# As of Jekyll 3.8.5

    def post_attr_hash(post_attr)
      # Build a hash map based on the specified post attribute ( post attr =>
      # array of posts ) then sort each array in reverse order.
      hash = Hash.new { |h, key| h[key] = [] }
      posts.docs.each do |p|
        p.data[post_attr].each { |t| hash[t] << p } if p.data[post_attr]
      end
      hash.each_value { |posts| posts.sort!.reverse! }
      hash
    end

    def tags
      post_attr_hash("tags")
    end

    def categories
      post_attr_hash("categories")
    end

So, we might as well just delegate to the methods in Jekyll::Site.
However, it's not much of a concern since Archives#tags and Archives#categories are called at most just once per call to the Archives#generate method.

--

*Notes:

  • I refrained from using extend Forwardable and def_delegators since we're dealing with just two methods here.
  • In Jekyll 4.0, Jekyll::Site#post_attr_hash is a memoized method. So there wont be additional hash allocation for each call to Archives#tags and Archives#categories.

@ashmaroli ashmaroli requested a review from a team April 17, 2019 17:30
@mattr-
Copy link
Member

mattr- commented Apr 17, 2019

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit f84c0e0 into jekyll:master Apr 17, 2019
jekyllbot added a commit that referenced this pull request Apr 17, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants