Skip to content
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

Sort collection items by an attribute. #5904

Closed
wants to merge 35 commits into from

Conversation

ashmaroli
Copy link
Member

@ashmaroli ashmaroli commented Feb 26, 2017

sort collection items by specifying a "sort_by" metadata key in the _config.yml and assigning the key a value that corresponds to a key set in the document's Front Matter.

Notes:

  • all items in the collection must have the key defined.
  • defaults to original sort! if none of the items have the key defined.

Example

# _config.yml

collections:
  tutorials:
    output: true
    sort_by: index


# _tutorials/get-started.md
---
title: Get Started
index: 1
---

# _tutorials/be-confident.md
---
title: Be Confident
index: 2
---
# default_docs_array => 
[#<Jekyll::Document _tutorials/be-confident.md collection=tutorials>, #<Jekyll::Document _tutorials/getting-started.md collection=tutorials>]

# docs_array_with_custom_sort => 
[#<Jekyll::Document _tutorials/getting-started.md collection=tutorials>, #<Jekyll::Document _tutorials/be-confident.md collection=tutorials>]

Tests will be added later.

/cc @jekyll/ecosystem

@gvwilson, this implementation may not qualify for your bounty, but in theory should sort the documents and assign the prev / next pagers accordingly.

Fixes #5754

sort collection items by specifying a "sort_by" metadata key in
the _config.yml and assigning the key a value that corresponds to a key
set in the document's Front Matter.
Copy link
Member

@pathawks pathawks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests will be added later.

@gvwilson
Copy link

@ashmaroli thanks for the quick turnaround, but this still requires people to modify multiple files when they want to reorder a couple of files or insert a new one. For example, if the collection includes alpha.md, beta.md, and gamma.md with ordering keys 1, 2, and 3, and I need to change the order to ['beta', 'gamma', 'alpha'], I have to edit three files (correctly) instead of just _config.yml.

@ashmaroli
Copy link
Member Author

ashmaroli commented Feb 26, 2017

but this still requires people to modify multiple files when they want to reorder a couple of files or insert a new one

@gvwilson True. Hence the reason I said that this implementation might not qualify for the bounty.
I chose this method because the array in the _config.yml is an Array of Strings while the Collection's array is an Array of Jekyll::Documents

@ashmaroli
Copy link
Member Author

@pathawks I added a basic test step. Will add more if you think this implementation could be beneficial addition to the Core..

@pathawks
Copy link
Member

I don't think that sorting by an index in a documents front matter is very valuable. What happens if I want to add a new page between 1 and 2? Do I have to edit the index of every page?

I'm not sure what problem this is solving.

@ashmaroli
Copy link
Member Author

@pathawks I've added an additional implementation to have the documents array modified by an order metadata as @gvwilson specified. I think the latest implementation is a bit inferior in performance in comparison to the earlier. So included both implementations in the PR.

sort_by can be used with stable collections where the items can be programmatically sorted by specifying a key defined in all the items.

order can be used for greater control with the arrangements of the array:

  • the order metadata should be an Array of just the filenames of the documents.
  • the items can be arranged in any desired order and the order will be virtually mapped to the document's sort_index Front Matter key, which will be used consequently to sort the array. Hence configuring via this metadata is slower than the former.
  • order metadata takes precedence over sort_by and will gracefully warn the user if a provided filename doesn't exist in the collection.

will add more tests, if required, after further feedback.

Copy link

@gvwilson gvwilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation and tests look good to me, but handling of "extra" files in collection doesn't appear specified or tested (but my Ruby knowledge is weak, so I may be missing something).

tutorials:
output: true
order:
- getting-started.md
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These appear to be in the same order as defined by the internal header's lesson value, so there's no way to tell whether the sorting is due to the config information or the internal data. Can this test be modified to put the files in a different order so that we can be sure they're being sorted by config spec?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the collection would be sorted by lesson key only if sort_by key is set to "lesson" here. I'll nevertheless edit the order in the test to bring in clarity.

end
end
docs.sort_by! { |d| d.data["sort_index"] }
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. What happens to files that are in the collection directory but not specified in the explicit order list in config?
  2. What happens to files that are in the collection directory but do not specify the key being used for sorting (in this case, don't have a lesson key)?

Copy link
Member Author

@ashmaroli ashmaroli Mar 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens to files that are in the collection directory.. ..don't have a lesson key?

The build process aborts with an Error.
I'll push a couple of patches to better handle such situations.

@gvwilson
Copy link

gvwilson commented Mar 3, 2017 via email

@pathawks
Copy link
Member

pathawks commented Mar 3, 2017

What happens to files that are in the collection directory.. ..don't have a lesson key?

The build process aborts with an Error.

This seems like it makes things complicated.

people may want to append them to the collection in natural (alphabetic) order

This seems like it makes things complicated.

@gvwilson
Copy link

gvwilson commented Mar 3, 2017 via email

If the user leaves out a certain document from the `order` array, its
`sort_index` value will equal the total no. of documents in the collection.

The value will be same for all documents left out and such documents will
be sorted by the default algorithm.
@ashmaroli
Copy link
Member Author

This is how things will roll now:

  • if there's a file that doesn't have the FrontMatter key specified for sort_by, the custom sort rolls back to the default sorting with a graceful warning (for each deviant file).
  • if there's a file missing in the order list, the file will be included after the custom list defined in order. If there's multiple files missing in the list, then the deviant files will appear after the custom list, in alphabetical order.

end.map!(&:last)
end

def sort_mapped_items(apples, oranges)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this helper method doesn't make sense because the input and output is both created and destroyed inside another method. It make more sense for all use of this helper array to be in the sort_docs_by_key! method. I'd like to see it all in one place to prevent confusing indirection.

@parkr
Copy link
Member

parkr commented Sep 8, 2017

Ruby 2.1.10 consistently fails here :( https://travis-ci.org/jekyll/jekyll/jobs/271100330

@ashmaroli
Copy link
Member Author

Ruby 2.1.10 consistently fails here

... and also on AppVeyor @ Ruby2.4.x ...
Feel free to take this PR off the queue to Milestone v3.6

@parkr parkr modified the milestones: v3.7.0, v3.6.0 Sep 11, 2017
@parkr
Copy link
Member

parkr commented Sep 11, 2017

Sorry, @ashmaroli 😦 I don't know why it's failing! Maybe something with the ! methods' return values changed in Ruby 2.1?

@gvwilson
Copy link

gvwilson commented Mar 11, 2018

Can someone please take another look at this - it would greatly simplify our lives to have it merged. cc @parkr @ashmaroli @pathawks @DirtyF

@ashmaroli
Copy link
Member Author

@gvwilson Over the course of time, the branch changed ideas and is now different from your original feature-request.. Even with this PR (as it stands currently) being merged in you'll not be able to rearrange collection documents manually by listing documents in the config file.

@ashmaroli ashmaroli closed this Mar 12, 2018
@gvwilson
Copy link

That's really unfortunate: the bounty I paid was precisely so that collection documents' order could be specified in the configuration file.

@ashmaroli
Copy link
Member Author

@gvwilson The Core Team is not willing to incorporate the proposed implementation into Jekyll Core.
The pamphlet associated with the bounty did state that the contributor will not be held responsible if
the PR would not get merged.

Besides, you need not wire your site's source to GitHub Pages to get it built on the fly, there are other alternatives like Netlify.com that allow you to use custom plugins and gems in the build sessions.

@jekyll jekyll locked and limited conversation to collaborators Dec 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specify order of collection items and set previous/next links accordingly
6 participants