search

dunst-project / dunst

Lightweight and customizable notification daemon
https://dunst-project.org
Other
4.56k stars 340 forks source link

Store colors in a struct #1306

Closed bynect closed 6 months ago

bynect commented 7 months ago

I changed every usage of colors from a string to a color struct. Colors are now parsed only once in the option_parser instead of every single draw call.

Note: Hopefully I haven't missed anything. I would be super grateful if someone could do some extra testing on their own if they encounter anything strange

bynect commented 7 months ago

Still working on the tests

bynect commented 7 months ago

Fixes what we talked about in #1286

bynect commented 7 months ago

Note that invalid colors sent to dbus are ignored

fwsmit commented 7 months ago

Yeah, looks good!

bynect commented 6 months ago

I will do some fixes and merge it then 👍

zappolowski commented 6 months ago

I will do some fixes and merge it then 👍

I wouldn't merge as long as CI doesn't work properly. I also accidentally merged a branch yesterday - but luckily, this was just update of a pod file and thus the chance of breakage is rather low.

bynect commented 6 months ago

Right, ci is still down 😬

zappolowski commented 6 months ago

@bynect As I've merged the new pipeline you could rebase this branch and then hopefully we can go forward merging it.

codecov-commenter commented 6 months ago

Codecov Report

Attention: Patch coverage is 83.91608% with 23 lines in your changes are missing coverage. Please review.

Project coverage is 65.30%. Comparing base (15b1e8c) to head (6740fb0). Report is 2 commits behind head on master.

Files Patch % Lines
src/draw.c 53.57% 13 Missing :warning:
src/notification.c 69.23% 4 Missing :warning:
src/option_parser.c 92.85% 3 Missing :warning:
src/dbus.c 75.00% 2 Missing :warning:
src/rules.c 87.50% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1306 +/- ## ========================================== + Coverage 65.06% 65.30% +0.23% ========================================== Files 48 48 Lines 8173 8215 +42 ========================================== + Hits 5318 5365 +47 + Misses 2855 2850 -5 ``` | [Flag](https://app.codecov.io/gh/dunst-project/dunst/pull/1306/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/dunst-project/dunst/pull/1306/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project) | `65.30% <83.91%> (+0.23%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bynect commented 6 months ago

@zappolowski should I merge it now?

zappolowski commented 6 months ago

Yes, just go ahead and merge it.