Skip to content

Conversation

@fwsmit
Copy link
Member

@fwsmit fwsmit commented Jun 30, 2021

This has been suggested in https://gitlab.freedesktop.org/xdg/xdg-specs/-/issues/77 to prevent name collisions.

@tsipinakis
Copy link
Member

Valgrind seems to be complaining. n1 isn't beeing freed for some reason?

@codecov-commenter
Copy link

codecov-commenter commented Aug 29, 2021

Codecov Report

Merging #886 (d0f055e) into master (9f4f110) will increase coverage by 2.42%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #886      +/-   ##
==========================================
+ Coverage   58.07%   60.50%   +2.42%     
==========================================
  Files          37       39       +2     
  Lines        6104     6228     +124     
==========================================
+ Hits         3545     3768     +223     
+ Misses       2559     2460      -99     
Flag Coverage Δ
unittests 60.50% <100.00%> (+2.42%) ⬆️

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

Impacted Files Coverage Δ
src/queues.c 91.49% <100.00%> (+0.44%) ⬆️
test/queues.c 98.76% <100.00%> (+0.10%) ⬆️
src/settings.c 56.52% <0.00%> (-1.82%) ⬇️
src/option_parser.c 83.87% <0.00%> (-1.53%) ⬇️
src/dunst.c 9.48% <0.00%> (-0.34%) ⬇️
src/notification.c 58.71% <0.00%> (-0.16%) ⬇️
src/x11/x.c 2.21% <0.00%> (-0.08%) ⬇️
src/input.c 0.00% <0.00%> (ø)
test/queues.h 100.00% <0.00%> (ø)
test/notification.c 100.00% <0.00%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f4f110...d0f055e. Read the comment docs.

@fwsmit
Copy link
Member Author

fwsmit commented Aug 29, 2021

@tsipinakis It was a problem with a notification_ref too many. I don't yet understand completely, but it's just a test so if valgrind doesn't complain it's fine. This PR should be ready to merge now

@fwsmit fwsmit merged commit 3055efd into dunst-project:master Sep 6, 2021
@fwsmit fwsmit deleted the small-fixes branch September 6, 2021 13:12
@fitrh
Copy link

fitrh commented Sep 27, 2021

Hi, how can i achieve the same result as below configuration after this commit ? Before this, i can set different state for each low, mid, and high volume but still stacking by using same stack tag, but now it seems same appname required for make it stacking

[volume]
    appname = "Audio"
    format = " <span font='32'>%s <span foreground='#a9b1d6'>%b</span>%%</span>"
    foreground = "#0db9d7"
    frame_color = "#0db9d7"
    highlight = "#0db9d7"
    history_ignore = yes
    new_icon = NONE

[volume-high]
    appname = "Audio High"
    format = " <span font='32'>%s <span foreground='#a9b1d6'>%b</span>%%</span>"
    foreground = "#ff7a93"
    frame_color = "#ff7a93"
    highlight = "#ff7a93"
    history_ignore = yes
    new_icon = NONE

[volume-low]
    appname = "Audio Low"
    format = " <span font='32'>%s <span foreground='#a9b1d6'>%b</span>%%</span>"
    foreground = "#a9b1d6"
    frame_color = "#a9b1d6"
    highlight = "#a9b1d6"
    history_ignore = yes
    new_icon = NONE

[volume-mute]
    appname = "Audio Mute"
    format = " <span font='32'>%s %b%%</span>"
    foreground = "#444b6a"
    frame_color = "#444b6a"
    highlight = "#444b6a"
    history_ignore = yes
    new_icon = NONE

@fwsmit
Copy link
Member Author

fwsmit commented Sep 28, 2021

Hi, how can i achieve the same result as below configuration after this commit ? Before this, i can set different state for each low, mid, and high volume but still stacking by using same stack tag, but now it seems same appname required for make it stacking

It's expected that this doesn't work anymore. There are multiple ways to fix this:

  • Provide all information for the notification properties via the notification command:
    dunstify -h string:fgcolor:#444b6a ...
    You should set the same appname. The markup can only be changed in the body (if this is a problem, make an issue about setting the format via hints).
  • Overwrite the notification by id:
    dunstify -r $ID
    You can keep the other things you did. Keep in mind this is kind of hacky.

@fitrh
Copy link

fitrh commented Sep 28, 2021

  • Provide all information for the notification properties via the notification command:
    dunstify -h string:fgcolor:#444b6a ...

Ah, I see, thank you.

@xlucn
Copy link

xlucn commented Oct 21, 2021

I am also confused. How do I manually stack notifications without knowing the appid? Setting the same appname does not result in the same appid.

Of course, the workaround involving the appid is to print the appid with dunstity -p and use that id for next notification. But that requires some scripting and doesn't work with general tools such as notify-send.

@fwsmit
Copy link
Member Author

fwsmit commented Oct 21, 2021

I am also confused. How do I manually stack notifications without knowing the appid? Setting the same appname does not result in the same appid.

You are only supposed to stack notifications coming from the same application, i.e. with the same appid. To stack notifications from the same applications, you just need to give them the same stack_tag. You can use dunst -print to see information about the notification to make rules to match only those notifications.

This PR only changes that the appid's of the notifications have to match to avoid name collisions in stack tags. What's your use case, since I don't see how it's affected by this PR.

@xlucn
Copy link

xlucn commented Oct 21, 2021

Sorry, I was mistaking the appid and notification id (I thought there was only one id-thing). The behavior did change for me after 1.7.0, but shouldn't be related to this PR. Please forget about that comment.

@fwsmit
Copy link
Member Author

fwsmit commented Oct 21, 2021

Ok you can open an issue about it then :)

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.

5 participants