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

Grouping criteria #111

Merged
merged 20 commits into from Feb 16, 2019

Conversation

Projects
None yet
2 participants
@vilhalmer
Copy link
Collaborator

vilhalmer commented Feb 13, 2019

Part 2 of 2! Closes #66. Also makes unicorns real.

Actual features:

  • Renamed group to group-by and made it available on the cli.
  • Grouping defaults to the new none criteria spec which never matches any notifications. This cannot be used in criteria sections because it would just be a fancy comment.
  • Add an invisible style option to skip rendering arbitrary notifications. This is not exposed on the cli as it would make mako fairly useless. Maybe after we add history?
  • Add %g format specifier for the group size.
  • Inserts two new default criteria into the list:
    • Set the format of all grouped notifications to include the group size and make them invisible.
    • Re-visibilize the first one in the group.
  • Groups count as a single notification when figuring out which ones to hide after max-visible.

Bonus feature:

  • Store the raw criteria strings on criteria like sway does so they're available for easy reading when debugging.

Known issues:

  • If multiple notifications per group are configured to be visible, only the first will be visible anyway when it is directly before the max-visible cutoff. I'm just going to file an issue for this since I don't expect many people to run into it.

Testing done:

  • Lots, also probably not enough.
  • Priority sorting still seems to work correctly, which I was worried about breaking.
  • Did a bunch of reconfiguration stress testing to make sure things regrouped correctly and nothing exploded.

Things not done:

  • I punted on converting hidden to a group. It's really hard to count the number of notifications outside of render. I still think this can be done, but it will take some more thought.
  • I didn't end up adding a greater-than operator for criteria. I have it written, but it was getting complicated enough on its own that I left it out.

vilhalmer added some commits Dec 1, 2018

Rename hidden style to invisible
Colliding with the existing hidden concept was way too confusing and
would lead users to think the hidden criteria should match invisible
notifications, which it does not.
@emersion
Copy link
Owner

emersion left a comment

Here are a few nits. Looks very good otherwise, nice job!

Show resolved Hide resolved include/criteria.h Outdated
Show resolved Hide resolved criteria.c Outdated
Show resolved Hide resolved notification.c Outdated
@emersion
Copy link
Owner

emersion left a comment

🌈

@emersion emersion merged commit 81b0be5 into emersion:master Feb 16, 2019

1 check passed

builds.sr.ht: .build.yml builds.sr.ht job completed successfully
Details
@emersion

This comment has been minimized.

Copy link
Owner

emersion commented Feb 16, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.