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

Only inherit themes which are really needed #458

Open
alexxcons opened this issue Jun 2, 2024 · 2 comments · May be fixed by #461
Open

Only inherit themes which are really needed #458

alexxcons opened this issue Jun 2, 2024 · 2 comments · May be fixed by #461

Comments

@alexxcons
Copy link

Hey @newhoa 👋

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 🙂

Currently, elementary-xfce has:

Inherits=elementary,Adwaita,gnome,hicolor

While the elementary-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
Copy link
Contributor

newhoa commented Jun 2, 2024

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
Copy link
Author

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!

newhoa added a commit to newhoa/elementary-xfce that referenced this issue Jun 15, 2024
The FreeDesktop.org Icon Theme Spec has been
updated stating that inherited themes should
be present on the system (considered dependencies).

The currently listed themes in elementary-xfce's
`Inherits` line are either no longer developed and
missing modern icons (gnome), or have reduced the scope
of their theme to focus on their OS (Adwaita, elementary).

elementary-xfce should currently be robust enough that
it should already include the icons found in these themes.
So it no longer seems very necessary to keep these, and
removing them will prevent unnecessary themes from being
forced as dependencies for users who may not want them.

Fixes shimmerproject#458
@newhoa newhoa linked a pull request Jun 15, 2024 that will close this issue
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 a pull request may close this issue.

2 participants