Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting min_icon_size = 0 disables recursive icon search #1094

Open
pmattern opened this issue Jul 28, 2022 · 12 comments
Open

Setting min_icon_size = 0 disables recursive icon search #1094

pmattern opened this issue Jul 28, 2022 · 12 comments
Assignees
Milestone

Comments

@pmattern
Copy link

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.

@fwsmit
Copy link
Member

fwsmit commented Jul 29, 2022

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?

@pmattern
Copy link
Author

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).

@fwsmit
Copy link
Member

fwsmit commented Jul 29, 2022

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.

@cyberpunkrocker-zero
Copy link

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.

@sahinakkaya
Copy link

sahinakkaya commented Nov 2, 2022

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...

@MikeWalrus
Copy link

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. :)

@useredsa
Copy link

useredsa commented Aug 8, 2023

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

@fwsmit
Copy link
Member

fwsmit commented Aug 19, 2023

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?

@useredsa
Copy link

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.

@fwsmit
Copy link
Member

fwsmit commented Aug 31, 2023

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.

@bynect
Copy link
Member

bynect commented Mar 2, 2024

@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.

@fwsmit
Copy link
Member

fwsmit commented Mar 2, 2024

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.

@bynect bynect self-assigned this Nov 4, 2024
@bynect bynect added this to the v2.0 milestone Nov 4, 2024
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

No branches or pull requests

7 participants