-
Notifications
You must be signed in to change notification settings - Fork 23
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
Display oldest notifications first by default #18
base: master
Are you sure you want to change the base?
Conversation
I made the notification sort more flexible, it is now possible to sort notification on multiple fields. Sort order is indicated in Another useful sort order could be |
LGTM but this should be documented somewhere other than here :-) [suggestion: Website + config template] |
Good point, I created regolith-linux/website#160 for website documentation. I'm not sure what a config template is. |
https://github.com/regolith-linux/regolith-i3-gaps-config/blob/master/config I think have the default there listed explicitly is a good idea. |
This is the i3 config file, I do not change anything there, only Xresources. |
I know; I suggest that you do actually modify that file so that it contains the default for the new setting. |
rofication/_server.py
Outdated
for sort_field in sort_fields: | ||
reverse = sort_field.startswith("!") | ||
sort_field = sort_field.lstrip("!") | ||
messages.sort(key=lambda message: message.asdict()[sort_field], reverse=reverse ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will crash if there's a typo in the field and it doesn't exist in the message.
A possibility would be, rather than crashing, returning a list with a single message reporting the error (something like "Field XX does not exist, check i3xrocks_notify_sort_by in XResource. Valid fields are....)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the code. Now if an incorrect field name is used, a message indicating that is added at the top of the list and the rest of the sorting is done normally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @dferrand , very nice PR. Can you explain a bit about how you tested this change? How will users know what values they can provide for the sort field?
r
I tested manually on my system by killing the "normal" regolith-rofication process and launching the dev one instead. Technically you can sort on any field of the Notification class (defined in rofication/_notification.py) but I don't see much practical use to sort on anything but timestamp and urgency. How would you suggest to let the user know the possible fields? |
Sorry for the delay.
We'll just need to document it on the website. This is the page and this is the markdown file that can include the details |
In response to regolith-linux/regolith-desktop#607 I changed rofication-daemon to reverse notification list order (to display the newest notification first) by default.
Previous behaviour (oldest notification first) can be restored by setting
i3xrocks.oldest.first
totrue
in Xresources.