Support ordering pages and articles when iterating in templates. #1348
Conversation
Order can be set to a metadata attribute or a sorting function. Default to order by slug for articles and order by filename for pages.
…y' into davidmarble-page-order-by Conflicts: docs/settings.rst pelican/generators.py pelican/tests/test_generators.py
|
The page ordering behaved precisely as expected in my (admittedly limited) testing. Thank you for bringing this up-to-date, Vincent! |
|
Very cool. Thanks for pulling this together! Sorry I couldn't find the time to help out with this. |
| the articles_page.object_list template variable is | ||
| ordered by slug. If you modify this, make sure all | ||
| articles contain the attribute you specify. You can | ||
| also specify a sorting function. |
There was a problem hiding this comment.
the call signature of the sorting function should be described
There was a problem hiding this comment.
If I understood the code below correctly, the function is supposed to be a key getter, not an actual sorting function, so calling it a sorting function is confusing.
The text about the sort-by-key function comes from: https://docs.python.org/2/library/stdtypes.html#sequence-types-str-unicode-list-tuple-bytearray-buffer-xrange Minor style cleanup as well.
|
If @vjousse could pull from my branch, I believe that I have addressed all of the code review comments. |
|
Here you are! |
|
@smartass101 @justinmayer hey guys, what do you think of the changes ? It would be great if this pull request could be merged. |
| elif order_by == 'basename': | ||
| index.sort(key=lambda x: os.path.basename(x.source_path or '')) | ||
| elif order_by != 'slug': | ||
| index.sort(key=attrgetter(order_by)) |
There was a problem hiding this comment.
I think this needs a try/except in case an invalid attribute is given.
There was a problem hiding this comment.
@avaris so something like:
try:
index.sort(key=attrgetter(order_by))
except AttributeError:
logger.error('There is no "{}" attribute in the metadata for "{}"'.format(order_by, slug))There was a problem hiding this comment.
@malept maybe warning is more appropriate since it'll fall back to slug. Which reminds me, pages with garbage order_by falls back to slug, not basename.
|
@vjousse I updated my branch again. |
|
@malept thanks I've updated the PR |
|
@smartass101 @justinmayer @avaris is everything fine for you? |
|
Very sorry for the long delay, Vincent. I've marked your pull request with the 3.5 milestone, so we'll be sure to review it before we issue the next release. Thanks for your patience! |
|
Better late than never ;) |
|
Looks good. Thanks to everyone for their contributions and their patience. Nice to finally deliver a feature that was requested two years ago. (^_^) |
Support ordering pages and articles when iterating in templates.
|
@vjousse: Looks like there's a test failure after merging this pull request. Would you do us a favor and see if you could determine and submit an appropriate fix? |
Fix for broken test in #1348
|
@justinmayer it seems that @avaris has already fixed the test :) |
I've merged #836 from @davidmarble with current master and added the tests from @malept fork.
By default pages are sorted by filename and articles by slug. It can be changed with ARTICLE_ORDER_BY and PAGE_ORDER_BY settings.