dunst-project / dunst

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

[RFC] config reload mechanism #1350

Open bynect opened 2 months ago

bynect commented 2 months ago

I wanted to add a way to reload the config files. part of the code is taken from #968.

In respect to #968 I added the ability to pass a list of config files to use to the dbus method. If none are passed it will use the old config list.

I noticed that the cmdline parser ignores multiple -conf options (without any acknowledgment of the fact). So I changed the behavior to accept a list of files.


Summary

bynect commented 2 months ago

@zappolowski it seems like the ci is having a problem with arch

maybe it's https://bbs.archlinux.org/viewtopic.php?id=276422?

codecov-commenter commented 2 months ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 41.74312% with 127 lines in your changes missing coverage. Please review.

Project coverage is 65.38%. Comparing base (20033b8) to head (bbf6d51). Report is 2 commits behind head on master.

Files Patch % Lines
src/rules.c 35.71% 63 Missing :warning:
src/dunst.c 0.00% 24 Missing :warning:
src/wayland/wl.c 0.00% 14 Missing :warning:
src/dbus.c 47.36% 10 Missing :warning:
src/queues.c 0.00% 9 Missing :warning:
src/option_parser.c 83.33% 3 Missing :warning:
src/settings.c 66.66% 3 Missing :warning:
src/draw.c 0.00% 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 #1350 +/- ## ========================================== - Coverage 66.08% 65.38% -0.71% ========================================== Files 50 50 Lines 8247 8345 +98 Branches 958 1006 +48 ========================================== + Hits 5450 5456 +6 - Misses 2797 2889 +92 ``` | [Flag](https://app.codecov.io/gh/dunst-project/dunst/pull/1350/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/1350/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project) | `65.38% <41.74%> (-0.71%)` | :arrow_down: | 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 2 months ago

mostly done. I'll do some more checks and maybe add a functional test

fwsmit commented 2 months ago

Thanks for picking this up! I never got it to work properly when I tried it. But when the code is structured well, it should not be too hard to implement

bynect commented 1 month ago

Thanks for picking this up! I never got it to work properly when I tried it. But when the code is structured well, it should not be too hard to implement

I made some updates. The only thing I am a bit unsure is the reapply of rules

bynect commented 1 month ago

Basically I added a rule struct inside the notification that is allocated and filled when we try to apply a rule to change a value. Then when we reload we reapply that rule to "revert" the original state. it should work but I haven't tested much (it is kind of involved)

@fwsmit does this solve the problem you said in the comments?

bynect commented 1 month ago

arch ci is not working ...

bynect commented 4 days ago

@zappolowski I made the changes you suggested. Did you try hot reloading and found any problem? I wanted to merge this soon