Closed elamperti closed 10 months ago
Merging #1150 (57bcb82) into master (5ad645c) will increase coverage by
0.00%
. The diff coverage is50.00%
.
:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more
@@ Coverage Diff @@
## master #1150 +/- ##
=======================================
Coverage 66.03% 66.03%
=======================================
Files 46 46
Lines 7595 7607 +12
=======================================
+ Hits 5015 5023 +8
- Misses 2580 2584 +4
Flag | Coverage Δ | |
---|---|---|
unittests | 66.03% <50.00%> (+<0.01%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
test/notification.c | 100.00% <ø> (ø) |
|
src/notification.c | 60.77% <50.00%> (+0.16%) |
:arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
Thanks for submitting this PR
Should we use a single format token (e.g. %X) to show all possible indicators in it? (actions, URLs, duplicates)
I don't think this is necessary. If you have a use case, then I wouldn't be against it.
How could this deprecate the old implementation? (or how should they live together to avoid duplicate indicators?)
show_indicators
is not really useful anymore with this implementation, so it should be marked deprecated. Then we could remove it in the next major version (which will probably not come any time soon). In the mean time, show_indicators
should just prepend (%A%U)
or something similar to the notification text, before it's being processed by your new code. If you do that right, nothing should change for existing users, but they are able to change the indicator after that.
hide_duplicate_count=false
should also just append (%D)
or similar to the notification text.
Hi, have you looked at my above comment? Let's figure out together what to do to finish this PR
Hi, sorry for leaving this PR hanging. I'll get back to it this week. Thank you!
Hey, thanks for your update. Let me know when you get to it!
It would be nice to add a format parameter for the number of actions
Yeah, that should definitely be possible
For organizational purposes I'm going to close this pull request. If you or anyone else wants to work on this, feel free to reopen this pull request or start a new one (referencing this PR).
Sorry for dropping the ball. I've just opened a new PR with the missing changes + added the option to show action count using %C
.
This PR introduces two new settings (
action_indicator
andurl_indicator
) which could be set to any string, and then be placed wherever the user finds it useful in theformat
string (using%A
or%U
).There's also the possibility to use
%D
in the format to show how many duplicates a given notification has.I've tested this locally using emoji instead of the classic "(A)" and "(U)". It works, so it should be usable with nerd fonts and similar icon fonts too.
ℹ️ As it is now, this PR is a draft to gather feedback. Things that are still to be done or defined:
%X
) to show all possible indicators in it? (actions, URLs, duplicates)