Skip to content

Conversation

@mimre25
Copy link
Contributor

@mimre25 mimre25 commented Jun 5, 2025

This PR adds a new config option default_pause_level that allows setting a pause level per default.

With this new option it's possible to accomplish what I describe in #1484 ("pause" dusnt per default), but in a more general way.

Closes #1484

mimre25 added 3 commits June 5, 2025 10:04
This allows to turn off notification that are not deemed to be importent
per default via manipulating the pause level.
It was listed with all the "keyboarr shortcut" options, so I figured
that's not where it belongs.
@mimre25
Copy link
Contributor Author

mimre25 commented Jun 5, 2025

I'm not quite sure if/how I should test this.

Any suggestion @bynect?

@mimre25 mimre25 marked this pull request as ready for review June 5, 2025 08:07
@bynect
Copy link
Contributor

bynect commented Jun 5, 2025

Maybe i would put the pause after the startup notification.

Anyway for testing you could copy one of the suite in test/notification and check if the spawned notification are being blocked? Or you could just modify the suite that there is

@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2025

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

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 64.90%. Comparing base (c11063b) to head (068e4e9).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
src/dunst.c 0.00% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1487      +/-   ##
==========================================
- Coverage   64.91%   64.90%   -0.01%     
==========================================
  Files          51       51              
  Lines        9023     9024       +1     
  Branches     1048     1048              
==========================================
  Hits         5857     5857              
- Misses       3166     3167       +1     
Flag Coverage Δ
unittests 64.90% <0.00%> (-0.01%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mimre25
Copy link
Contributor Author

mimre25 commented Jun 5, 2025

Maybe i would put the pause after the startup notification.

I can do that - no problem.

Anyway for testing you could copy one of the suite in test/notification and check if the spawned notification are being blocked? Or you could just modify the suite that there is

I looked around in the tests but I couldn't quite find a test that checks anything similar to what I'm doing.

In test/setting.c there is test_dunstrc_defaults that seems to check if all settings with a default value are loaded correctly.
That'd cover the check for default_pause_level to be usable.

The tests in test/queue.c and test/notification.c test aspects of notifications and the queue including that they respect the pause level.
However, what they all have in common is that they have a narrow scope.
For example, test_queues_update_pause_level does insert notifications into the queue and then checks if they'd be shown with different pause levels.

However, the feature I'm adding is only taking affect if the dunst_main is called - which is not called in any test.
So to test it, I would need to somehow simulate running dunst (ie, dunst_main()) but then inspecting the state before the mainloop starts.

I don't quite know how I'd do that - if you want I can find out, but I'd argue that this might be a bit overblown given that the new settings only set the pause level.

What do you think?

@bynect
Copy link
Contributor

bynect commented Jun 5, 2025

Right. Maybe we should move that line of code somewhere else for testing. Maybe in the settings part. But for now it seems okay to put it there.

@mimre25
Copy link
Contributor Author

mimre25 commented Jun 13, 2025

@bynect do you still want/expect me to make changes here, or is this good to go? 🙂

@bynect
Copy link
Contributor

bynect commented Jun 13, 2025

good to go. shortly I'll pull.

@bynect bynect merged commit fb59a30 into dunst-project:master Jun 13, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pause per default

3 participants