Allow sorting by custom properties

[afeld]

Fixes #1802.

Added a failing test to support sort:'some-custom-property'. Have spent a couple hours poking around Jekyll/Liquid to figure out how to make those properties available to the filter, but haven't been able to figure it out yet.

Presumably a fix would work for other filters as well? Not sure how to test more generically.

P.S. These things are called "variables" in http://jekyllrb.com/docs/frontmatter/, but are called "properties" in the Liquid docs (e.g. in sort). Sorry for my inconsistency... I'll clean up later.

/cc @parkr @haacked

[afeld]

K, ready for 👀!

[coveralls]

Coverage decreased (-0.09%) when pulling c2b7504 on afeld:sort-attributes into 12a55b8 on jekyll:master.

[parkr]

My only worry with this is that then folks won't be able to sort by layout, name, dir, etc. data doesn't encase everything, unfortunately.

[afeld]

Not entirely true, since some of those will be covered by the respond_to?. I'll add some more tests and make #[] cover all of the attributes we could want.

[afeld]

K, made (most) other properties accessible via #[] now too. This gave me a lot of ideas about refactoring how data is passed around, but I'll leave those for separate PRs.

[parkr]

@afeld Awesome! Looks like Travis doesn't like your latest changes. Would you mind fixing up those broken tests? Thanks!

[afeld]

Fixed!

[parkr]

What do you think about this:

if respond_to?(property.to_sym)
  send(property.to_sym)
elsif respond_to?(:data)
  data[property]
else
  ""
end

Or something? What kind of precedent are you looking to take advantage of?

[afeld]

I was looking at the other uses of ATTRIBUTES_FOR_LIQUID, which I assume is a security measure so that arbitrary methods can't be called through Liquid.

[parkr]

Mm, the security vulnerabilities are definitely an important concern. What about using to_liquid.include? instead?

[afeld]

Because then it's needlessly constructing the whole hash, where only the keys and the single value are needed. I think there's a lot of refactoring that could be done around to_liquid, but I'm hoping to keep this PR from getting too complicated.

[parkr]

Looks pretty good to me. @mattr-, would love to get your 👀 on this.

[parkr]

I'm 👍 on this.

Just want @mattr-'s input then we'll be good to go.

[mattr-]

Looks fine to me. Although I beginning to think I'm the only person who still prefers hashrockets to the new ruby 1.9 syntax. 😃

[qrohlf]

This is in 2.0.0.rc1 latest, right? I've been struggling with trying to sort my chapters collection by a custom index property all morning, and if @afeld or anyone else could give me a nudge in the right direction I'd be infinitely grateful.

Here's what I've got right now:

(default.html template snippet)

{% assign contents = site.chapters | sort:'index' %}
{% for chapter in contents %}
  <a href="{{site.baseurl}}{{chapter.url}}" class="list-group-item">{{chapter.index}}. {{ chapter.subtitle }}</a>
{% endfor %}

[parkr]

Maybe @penibelst is experiencing the same issue with assign and sort? Ref: #2275

[doktorbro]

@qrohlf Is contents empty after assign?

[qrohlf]

@penibelst yeah, it's empty.

[doktorbro]

@qrohlf I would say the sorting feature is broken.