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

Sorting with priority #2201

Merged
merged 8 commits into from Jan 4, 2016
Merged

Conversation

@Determinant
Copy link
Contributor

@Determinant Determinant commented Dec 29, 2015

As discussed with ChrisWarrick, nikola currently implements sorting in nikola/nikola.py instead of the separated scan_posts plugin. Since the "sticky posts" is a very useful and desirable feature (at least for CMS and blog sites), I guess the most direct and simple way is to extend the original sorting key: (p.date, p.source_path) with an additional sorting key which can override the order. The problem is whether to directly inject the code into nikola.py or try to move the sorting functionality to an individual plugin or to scan_posts plugin. This PR demonstrates a possible way to incoporate into nikola.py and it works without breaking the original behavior. Users can optionally use .. priority: 1 to make a post sticky (which means the default is 0). They can also use other number to give a finer order control.

ignore=E501
ignore = E501

[easy_install]

This comment has been minimized.

@Kwpolska

Kwpolska Dec 29, 2015
Member

delete that

@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Dec 29, 2015

👍 from me

Correct indentation:

            thing.sort(key=lambda p:
                       (int(p.meta('priority')) if p.meta('priority') else 0,
                        p.date, p.source_path))

@felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Dec 29, 2015

There are some more places where posts are sorted, for example for tag pages, archives, ... Should these also respect priority? That would require some more changes.

How about having the Post object return a sort key? Then all places where posts are sorted could simply use that sorting key, and future changes to the sorting order can be done in one place (that function of Post).

@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Dec 29, 2015

Is sorting things multiple times a good thing to do in the first place?

@felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Dec 29, 2015

Of course, you could also rewrite a lot of code to ensure that resorting isn't needed.

Anyway, even if that is done, putting the sort key into a function of Post makes it cleaner in my opinion. Especially for plugin writers who want to do something fancy with sets of posts and simply want to sort the result, instead of writing a bunch of code which compares the set with the original timeline and reconstructs the correct order from there.

@Determinant Determinant force-pushed the Determinant:sorting-with-priority branch from 4ee60d2 to e5f2ca4 Dec 29, 2015
@Determinant
Copy link
Contributor Author

@Determinant Determinant commented Dec 29, 2015

Modified as suggested by @Kwpolska. @felixfontein, I agree with your idea. I'd definitively like to see the behavior of post sorting can be somehow customized. How do you plan to expose this functionality to users? Also via plugins? Since this commit is just a small change, you and @Kwpolska could adopt it as a temporary solution (with some TODO comments, if needed), but of course I'd like to see or implement a more elegant way of doing this. :)

@ralsina
Copy link
Member

@ralsina ralsina commented Jan 4, 2016

So, I don't know :-)

@Kwpolska if you +1 this, let's merge it. I prefer not to have PR rot (and we have some already)

Kwpolska added a commit that referenced this pull request Jan 4, 2016
@Kwpolska Kwpolska merged commit 8326cf7 into getnikola:master Jan 4, 2016
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants