Support sorting the collection in a for loop tag. #101

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
@KeeperPat

You can specify ascending or descending by doing order:ascending or order:descending. You can specify what attribute you want to search on by using the sort_by attribute. So "{% for item in collection sort_by:name order:descending %}" will iterate through the collection sorted by name in descending order.

Support sorting the collection in a for loop tag. You can specify asc…
…ending or descending by doing order:ascending or order:descending. You can specify what attribute you want to search on by using the sort_by attribute. So "{% for item in collection sort_by:name order:descending %}" will iterate through the collection sorted by name in descending order.
@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic Mar 27, 2012

Please! What else needs to happen for this to go in, I'll gladly help clean it up.

+1

mixonic commented Mar 27, 2012

Please! What else needs to happen for this to go in, I'll gladly help clean it up.

+1

@mixonic mixonic referenced this pull request Mar 27, 2012

Closed

Sort in for loop #58

@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic Mar 28, 2012

Arg, I cannot seem to get this working as expected with Jekyll. Aaarg.

mixonic commented Mar 28, 2012

Arg, I cannot seem to get this working as expected with Jekyll. Aaarg.

@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic Mar 28, 2012

I think because the sorting code isn't calling to_liquid on the data objects before sorting. Hm.

mixonic commented Mar 28, 2012

I think because the sorting code isn't calling to_liquid on the data objects before sorting. Hm.

@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic Mar 28, 2012

I had to add an additional elsif with to_liquid to sort pages in Jekyll:

elsif collection.first.respond_to?(:to_liquid) and collection.first.to_liquid.respond_to?('[]') and !collection.first.to_liquid[sort_property].nil?
  collection.sort {|a,b| "#{a.to_liquid[sort_property]}" <=> "#{b.to_liquid[sort_property]}" }

Also note wrapping the hash access in quotes so they become strings. That's needed to comparing nil values. Er, slightly nasty.

mixonic commented Mar 28, 2012

I had to add an additional elsif with to_liquid to sort pages in Jekyll:

elsif collection.first.respond_to?(:to_liquid) and collection.first.to_liquid.respond_to?('[]') and !collection.first.to_liquid[sort_property].nil?
  collection.sort {|a,b| "#{a.to_liquid[sort_property]}" <=> "#{b.to_liquid[sort_property]}" }

Also note wrapping the hash access in quotes so they become strings. That's needed to comparing nil values. Er, slightly nasty.

@lzap

This comment has been minimized.

Show comment
Hide comment
@lzap

lzap Jul 25, 2012

So what is the status? I really like this feature. Is there a workaround for github safe mode?

lzap commented Jul 25, 2012

So what is the status? I really like this feature. Is there a workaround for github safe mode?

@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic Jul 25, 2012

@lzap no real traction unfortunately. Both Liquid and Jekyll are pretty slow to get anything to happen.

I'm using a fork of Liquid in my GitHub-pages-compatible service at Spinto. You could use the Spinto Gem directly without the service at spintoapp.com if you just want a stable version of Jekyll that supports sorting for loops.

mixonic commented Jul 25, 2012

@lzap no real traction unfortunately. Both Liquid and Jekyll are pretty slow to get anything to happen.

I'm using a fork of Liquid in my GitHub-pages-compatible service at Spinto. You could use the Spinto Gem directly without the service at spintoapp.com if you just want a stable version of Jekyll that supports sorting for loops.

@jcemer jcemer referenced this pull request in braziljs/conf-boilerplate Sep 20, 2012

Closed

Dynamically generate Schedule's section #6

@yihui

This comment has been minimized.

Show comment
Hide comment

yihui commented Nov 8, 2012

+1

@dignifiedquire

This comment has been minimized.

Show comment
Hide comment

👍

@cobyism cobyism referenced this pull request in github/teach.github.com Mar 15, 2013

Closed

Order logic on Class Notes list #76

@chadokruse chadokruse referenced this pull request in chadokruse/chadokruse.github.com Jun 14, 2013

Closed

DRY up tiles #10

@fw42

This comment has been minimized.

Show comment
Hide comment
@fw42

fw42 Jun 18, 2013

Member

I'm not sure this is necessary. What's the advantage to using the sort filter (or to sorting stuff before it hits Liquid)? Also, this might cause a lot of confusion when used in combination with pagination outside of Liquid.

Member

fw42 commented Jun 18, 2013

I'm not sure this is necessary. What's the advantage to using the sort filter (or to sorting stuff before it hits Liquid)? Also, this might cause a lot of confusion when used in combination with pagination outside of Liquid.

@variousauthors

This comment has been minimized.

Show comment
Hide comment
@variousauthors

variousauthors Jul 30, 2013

+1

I'd like this feature and I think it has advantages. With the sort filter, one can only sort arrays.

{% assign sorted_collection = collection.products | sort:'created_at' %}

Won't work because collection.products is a drop not an array.

{% for product in collection.products sort_by:'created_at' %}

Would work, I hope. Am I wrong here? I'm pretty sure the sort filter is kind of useless at the moment, since the only way to create arrays seems to be calling split on a string.

+1

I'd like this feature and I think it has advantages. With the sort filter, one can only sort arrays.

{% assign sorted_collection = collection.products | sort:'created_at' %}

Won't work because collection.products is a drop not an array.

{% for product in collection.products sort_by:'created_at' %}

Would work, I hope. Am I wrong here? I'm pretty sure the sort filter is kind of useless at the moment, since the only way to create arrays seems to be calling split on a string.

@bsodmike

This comment has been minimized.

Show comment
Hide comment
@bsodmike

bsodmike Nov 9, 2013

+1 hope this is added soon!

bsodmike commented Nov 9, 2013

+1 hope this is added soon!

@cornae

This comment has been minimized.

Show comment
Hide comment

cornae commented Jan 17, 2014

+1

@bsodmike

This comment has been minimized.

Show comment
Hide comment
@bsodmike

bsodmike Jan 17, 2014

I wonder if this'll get merged in by 2015? It's already been 2-years in the waiting /facepalm

I wonder if this'll get merged in by 2015? It's already been 2-years in the waiting /facepalm

@bamos

This comment has been minimized.

Show comment
Hide comment

bamos commented Jan 26, 2014

+1

@arthurnn

This comment has been minimized.

Show comment
Hide comment
@arthurnn

arthurnn Jan 26, 2014

Contributor

I am ok into adding the sort tag. As we already have the limit and offset, having a sort make sense.
@KeeperPat Can you rebase our branch against master, and fix conflicts?

Contributor

arthurnn commented Jan 26, 2014

I am ok into adding the sort tag. As we already have the limit and offset, having a sort make sense.
@KeeperPat Can you rebase our branch against master, and fix conflicts?

@fw42

This comment has been minimized.

Show comment
Hide comment
@fw42

fw42 Jan 26, 2014

Member

@releod, @jduff, what do you guys think about this?

Member

fw42 commented Jan 26, 2014

@releod, @jduff, what do you guys think about this?

@fw42

This comment has been minimized.

Show comment
Hide comment
@fw42

fw42 Jan 26, 2014

Member

Seems like there is a lot of demand for this, at least outside of Shopify.

Member

fw42 commented Jan 26, 2014

Seems like there is a lot of demand for this, at least outside of Shopify.

@jduff

This comment has been minimized.

Show comment
Hide comment
@jduff

jduff Jan 26, 2014

Contributor

This is only useful when you have the full set in memory, which usually isn't the case (if it is you're probably asking for trouble).

I don't think this will be as useful as people expect, especially if they are wanting to use it with a web application at scale.

Contributor

jduff commented Jan 26, 2014

This is only useful when you have the full set in memory, which usually isn't the case (if it is you're probably asking for trouble).

I don't think this will be as useful as people expect, especially if they are wanting to use it with a web application at scale.

@fw42

This comment has been minimized.

Show comment
Hide comment
@fw42

fw42 Jan 26, 2014

Member

Yeah this is my concern as well. I don't think this will play nicely with pagination and will mostly cause confusion, at least with the current implementation.

Member

fw42 commented Jan 26, 2014

Yeah this is my concern as well. I don't think this will play nicely with pagination and will mostly cause confusion, at least with the current implementation.

@KeeperPat

This comment has been minimized.

Show comment
Hide comment
@KeeperPat

KeeperPat Jan 27, 2014

Updated in PR #304. I am no longer at LivingSocial so can't push to the original repository.

I also incorporated the change from @mixonic to support sort_by on attributes on an object that responds to to_liquid.

Updated in PR #304. I am no longer at LivingSocial so can't push to the original repository.

I also incorporated the change from @mixonic to support sort_by on attributes on an object that responds to to_liquid.

@KeeperPat KeeperPat closed this Jan 27, 2014

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