search

shimmerproject / elementary-xfce

Elementary icons forked from upstream, extended and maintained for Xfce
GNU General Public License v2.0
269 stars 37 forks source link

Only inherit themes which are really needed #458

Open alexxcons opened 4 weeks ago

alexxcons commented 4 weeks ago

Hey @newhoa :wave:

Now that the related xdg merge request got merged and v0.18 of the spec got released, I am starting my mission to get the "Inherits" attribute right for popular icon-themes to prevent potentially missing icons :slightly_smiling_face:

Currently, elementary-xfce has:

Inherits=elementary,Adwaita,gnome,hicolor

While theelementary-icon-theme does not have any Inherit entry at all (seems to be fd.org compliant by itself), the Xfce variant specifies a lot of items here. So I wonder why Adwaita and gnome are listed here.

For hicolor, there is no need to list it here, since according to the spec, it is anyhow searched as fallback by icon lookup implementations.

So if there is no strong reason to keep them in the Inherits list, I would suggest dropping at least Adwaita, gnome and hicolor.

If elementary-xfce is fd.org complete by its own, I would suggest to as well drop the complete Inherits section.

If one/some of the other themes need to stay on the list for some reason, it would be good to have a section in the README.md to explain packagers that the elementary-xfce package should have a hard dependency on the other listed themes.

newhoa commented 4 weeks ago

Nice on the xdg MR!

I'm all for dropping the Inherits line altogether. These were all added long ago, maybe when the theme was less complete. But I think at this point elementary-xfce is a more complete theme than any of the inherited themes, especially since Adwaita and elementary have significantly reduced the scope of their themes to focus on their DEs (and with Adwaita intentionally having moved away from being an FD.o theme now).

The only issue with dropping hicolor from Inherits is that not all apps use Gtk/Qt for their icon lookups and may not have properly implemented falling back to hicolor in their custom lookups. In these cases, they may depend on hicolor being listed as an inherited theme (I think this upstream issue might be an example of that). So it is possible that this might "break" more independent or older apps. Dropping it might be a way to push them to properly implement the spec in their icon lookup code, but it might be a burden on some devs and have less of a chance being fixed in less-maintained projects.

Anyway, thanks for the heads up! I'll make a PR and maybe ochosi and bluesabre can have a look at it.

alexxcons commented 4 weeks ago

Uh, I did not know that there are apps which implement yet other icon loading mechanism (I suppose it would be nice to offer a standardized XDG lib for this service)

So you might be right that it would be better to keep hicolor … would not be nice to break apps just for the sake of strict spec enforcement.

Just checked some other icon-themes: E.g. breeze and Papirus Inherit from hicolor as well. I suppose it will not hurt to just keep it.

Thank you for taking care on it!