dunst icon indicating copy to clipboard operation
dunst copied to clipboard

Setting min_icon_size = 0 disables recursive icon search

Open pmattern opened this issue 3 years ago • 12 comments

Title says it basically all.

Steps to reproduce:

  1. Make sure that icon theme Adwaita is available and Dunst is using the default configuration file.
  2. Launch a notification featuring an icon that's provided by the icon theme. The icon will show up.
  3. Set min_icon_size = 0, either within the default configuration file or a custom copy in $XDG_CONFIG_HOME/dunst. Restart Dunst.
  4. Repeat step 2. The icon won't show up any more.

Seems to be a regression which was introduced after release 1.8.1. Unfortunately, I lack the time to bisect this right now. Maybe related to #1086, but the differences warrant an issue on its own, IMHO.

1ef38e5 on Arch Linux x86_64 per AUR package dunst-git.

pmattern avatar Jul 28 '22 18:07 pmattern

The meaning of min_icon_size has changed slightly. It's now used for looking up icons in your icon theme. Since no icons have size 0, nothing gets found. So it's not a bug, but intended behaviour. Does this behaviour limit your configuration significantly?

fwsmit avatar Jul 29 '22 16:07 fwsmit

Not sure I understand what the new behavior is meant to be.
If min_icon_size = n is supposed to mean "look for icons of at least size n", then n=0 shouldn't fail. But if it's supposed to mean "look for icons of exactly size n", then wording min_icon_size is pretty misleading.

Whatever the new behavior is meant to be, you badly need to update the configuration file's comment. Wasting time due to plain wrong documentation really sucks, tbh.

That new behavior will probably never be an issue for me (wasting time figuring out why icons stopped showing up was, though).
But I recon many users won't like it - the purpose according to the current comment ("high-dpi") seems reasonable to me, and in #1086 you've already seen complaints about icons being too small (albeit for a different reason).

pmattern avatar Jul 29 '22 17:07 pmattern

It's looking for icons that can be scaled to a certain size according to the index.theme from an icon theme. It's not looking for bigger icons afterwards. The wording might not be the best but it makes sense in the instance where you have an icon not from your theme since it's scaled between those bounds.

If you think you can improve the documentation, go ahead and submit a pr or be more detailed than 'this sucks'.

I guess we'll hear from users if they have a problem with the new behavior. Until then let's not make it a problem.

fwsmit avatar Jul 29 '22 17:07 fwsmit

I was also caught in this... I had not updated my dunstrc for a while, and when I today moved to the new 'recursive-icon-search'-format (1.9.0), I just copied my old settings to the new dunstrc, including the "min_icon_size = 0" setting, and... "Oh, the recursive icon search is not working at all..."

The easiest fix is to just comment out the "min_icon_size" line.

There's no mention of the new (or old, either) behaviour of the "min_icon_size" in the documentation, and the comment in the default dunstrc is misleading, implying the old behaviour. Please fix this some way or other.

cyberpunkrocker-zero avatar Sep 10 '22 13:09 cyberpunkrocker-zero

I agree with @cyberpunkrocker-zero. It should give at least a warning when using min_icon_size with recursive_icon_search. I was debugging this problem for hours:

"Path of my theme is correctly searched. Why is it not finding the icon? Oh, its because this size parameter is 0 and it does not match with any theme. Why is it 0? Where does it come from? Hmm, it looks like it is using notification's min icon width property. Why is that 0? Apparently, notifications are created with "min_icon_size = 32" and somewhere in the middle, "notification->min_icon_size" is setting to 0." And at this point I was completely unaware of the existence of this setting in dunstrc. So I ended up tracing this property all the way down to "rule_apply" function only to realize that I AM THE ONE setting it in dunstrc. Sigh...

sahinakkaya avatar Nov 02 '22 01:11 sahinakkaya

What makes it more confusing is the comment: "set to 0 to disable". Maybe the implementation doesn't allow disabling this feature anymore? I think the solution might be allowing the scaling to be actually turned off, or changing the comment to state that it's always on. :)

MikeWalrus avatar Jan 07 '23 15:01 MikeWalrus

I agree with @pmattern that the setting name should be changed. It is completely misleading.

useredsa avatar Aug 08 '23 15:08 useredsa

The min icon size key was reused for the recursive icon search for backward compatibility, but I agree this isn't perfect. The reason why min and max icon size couldn't both be used is that the recursive icon search needs a specific icon size. So maybe a key icon_size should be introduced. Would that fix your issue?

fwsmit avatar Aug 19 '23 17:08 fwsmit

The min icon size key was reused for the recursive icon search for backward compatibility, but I agree this isn't perfect. The reason why min and max icon size couldn't both be used is that the recursive icon search needs a specific icon size. So maybe a key icon_size should be introduced. Would that fix your issue?

I am unaware of why the recursive icon search cannot search for a range of sizes or for scalable icons, but if that is the case, then having a key icon_size would make sense, yes.

useredsa avatar Aug 21 '23 11:08 useredsa

The recursive icon search follows the icon theme specification. The way it works is searching for icons that are compatible to a certain size according to your icon theme. So this can be a scalable icon or a raster icon that is exactly that size.

So it doesn't make sense to specify a range of icon sizes, since it only searches for one icon size.

fwsmit avatar Aug 31 '23 15:08 fwsmit

@fwsmit according to the dunstrc example min_icon_size = 0 disables it. So either we change the description or update the behavior in find_in_theme. I was thinking: if the user doesn't care about a minimum size, we should just get any scalable size icon. But I don't know if it goes against a protocol. It is however quite annoying at the moment this inconsistency between recursive icon lookup on and off.

bynect avatar Mar 02 '24 09:03 bynect

We can definitely think about a different way of configuring the icon size. The protocol doesn't say anything about how it should be configured. Only that each icon has a certain size range in which it works well. So it's best to let the user configure one icon size and find an icon that works in that size.

fwsmit avatar Mar 02 '24 10:03 fwsmit