dunst-project / dunst

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

Fix commandline handling #1208

Closed zappolowski closed 9 months ago

zappolowski commented 9 months ago

The output of dunst -h differs from the one of dunst --help due to how the usage string is constructed. Furthermore, the options -print and --startup_notification are not listed at all (due to incorrect ordering of options).

This PR refactors cmdline_find_option to handle more than two alternatives (which fixes the first issue, when calls to cmdline_get_* are merged) and reorders command line parsing in a way, that all currently available options are listed.

NB: I've added -startup_notification as alternative to --startup_notification as it was the only option only available with to hyphens and thus breaking consistency.

codecov-commenter commented 9 months ago

Codecov Report

Merging #1208 (d844e1d) into master (2cd719a) will decrease coverage by 0.01%. Report is 3 commits behind head on master. The diff coverage is 64.28%.

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

@@            Coverage Diff             @@
##           master    #1208      +/-   ##
==========================================
- Coverage   66.03%   66.02%   -0.01%     
==========================================
  Files          46       46              
  Lines        7595     7584      -11     
==========================================
- Hits         5015     5007       -8     
+ Misses       2580     2577       -3     
Flag Coverage Δ
unittests 66.02% <64.28%> (-0.01%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/option_parser.c 80.28% <100.00%> (-0.44%) :arrow_down:
src/dunst.c 11.01% <0.00%> (+0.27%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

fwsmit commented 9 months ago

That's a great find! It even simplifies the code :)

Could you add the new command line options to the dunst.1.pod man page?

zappolowski commented 9 months ago

Could you add the new command line options to the dunst.1.pod man page?

Just did ... and also fixed the spelling in the manpage for --startup-notification. Basically, I'd prefer to have it that way, but changing it would be an incompatible change.

fwsmit commented 9 months ago

Thanks for the improvements! This will be merged now