ui: Theme lookup should respect XDG_DATA_DIRS
mods, extensions and others GNOME Shell assets lookup into each dir, relative to XDG_DATA_DIRS. However, this isn't the case for themes (which can be referenced by a mod in a different XDG_DATA_DIR), hardcoding global.datadir.
The fix is to have the theme finding pattern following the same logic than other elements. Fixes #167 (closed).
-
-
changed the description
Toggle commit list -
254 254 if (stylesheet.query_exists(null)) 255 255 return stylesheet; 256 256 257 stylesheet = Gio.File.new_for_path(global.datadir + '/theme/' + name); 258 if (stylesheet.query_exists(null)) 259 return stylesheet; 257 let dataDirs = GLib.get_system_data_dirs(); -
Developer
Looks good, but... This should also include
global.datadir
to me, as it could point to some other prefix not included in XDG dirs per build settings. -
Thanks! I didn't saw cases where
global.datadir
wouldn't be included in the system_data_dirs, but I'm happy to add it as a fallback afterwards.
-
-
-
Rebased on latest master and pushed after including Marco's comment.
-
Developer
Looks good now, I can't merge this from gitlab, but feel free to rebase again :)
PS: If you allow edits on the MP I guess others devs can do it from gitlab itself too.
-
Master
The commit message has errors:
mods, extensions and others GNOME Shell assets
mModes, extensions and others GNOME Shell assets
lookup into each dir, relative to XDG_DATA_DIRS.
are searched in each directory in XDG_DATA_DIRS
Giving those paths higher priority than global.datadir.
This is not a sentence
The fix is to have the theme finding pattern following the same logic than other elements.
following the same logic asthan other elements.
(I'll also note that our stance has always been that themes are not officially supported (which is why there's no setting and users that absolutely must fiddle with the looks need to install an extension), and I don't want anybody to get the impression that this is changing)
-
added 126 commits
-
5b5d99b2...69afe778 - 125 commits from branch
GNOME:master
- c9feb51b - ui: Theme lookup should respect XDG_DATA_DIRS
Toggle commit list -
5b5d99b2...69afe778 - 125 commits from branch
-
Thanks for the commit review, applied and rebased on latest master!
I aree that themes aren't officially supported (and this is why there has been a lot of agreed css breakages in it, I guess). I don't think this gives that impression of official support more as XDG_DATA_DIRS is already searched through to look for
modes/….json
files. In addition, having those files living anywhere in XDG_DATA_DIRS and not the themes isn't coherent: the themes can be referenced viastylesheetName
key from the modes. Another way would be to only load the theme relative to selectedmodes/….json
mode.I think the best course of actions for now (to not break modes XDG_DATA_DIRS support), is continuing affirming themes aren't officially supported and respecting XDG_DATA_DIRS for them as well.
-
Master
I think the best course of actions for now, is continuing affirming themes aren't officially supported and respecting XDG_DATA_DIRS for them as well.
Agreed. My comment wasn't meant as an objection to the change, but as a clarification to avoid any misunderstandings (like, say, some "GNOME finally recognizes hard-working theme authors!11!!" headline on WorldOfGnome)
Modes, extensions and other GNOME Shell assets are search
searched
are search in each directory, relative to XDG_DATA_DIRS.
"each directory, relative to XDG_DATA_DIRS" sounds like "$XDG_DATA_DIRS/*", which isn't what is searched (and likely not even a valid path). If you want to be more specific than the previously proposed "each directory in XDG_DATA_DIRS", use something like "in appropriate subdirectories of each directory in XDG_DATA_DIRS".
However, this isn't the case for themes (which can be referenced by a mod in a different XDG_DATA_DIR).
mode
We give a higher priority to those paths before fallbacking to global.datadir.
fallbacking -> falling back
This is still unclear, as the preceding sentence talks about themes, for which global.datadir is the only path rather than a fallback. Assuming you are talking about non-theme assets, this would be better as an extension of the first sentence:
"[...] XDG_DATA_DIRS, falling back to global.datadir if necessary. However that isn't the case for themes, which are currently always expected in global.datadir, even when referenced by a mode in a different XDG_DATA_DIR."
-
-
Agreed. My comment wasn't meant as an objection to the change, but as a clarification to avoid any misunderstandings (like, say, some "GNOME finally recognizes hard-working theme authors!11!!" headline on WorldOfGnome)
Haha, indeed (or some other similar websites) ;)
"each directory, relative to XDG_DATA_DIRS" sounds like "$XDG_DATA_DIRS/*", which isn't what is searched (and likely not even a valid path). If you want to be more specific than the previously proposed "each directory in XDG_DATA_DIRS", use something like "in appropriate subdirectories of each directory in XDG_DATA_DIRS".
This sounds a little bit like repetitive but more precised, agreed.
This is still unclear, as the preceding sentence talks about themes, for which global.datadir is the only path rather than a fallback. Assuming you are talking about non-theme assets, this would be better as an extension of the first sentence:
"[...] XDG_DATA_DIRS, falling back to global.datadir if necessary. However that isn't the case for themes, which are currently always expected in global.datadir, even when referenced by a mode in a different XDG_DATA_DIR."
Agreed, changed to this.
I just pushed the new changelog with those changes, thanks!
-
Master
LGTM, but the branch got behind master and I don't have permissions to rebase ...
-
256 256 if (stylesheet.query_exists(null)) 257 257 return stylesheet; 258 258 259 let dataDirs = GLib.get_system_data_dirs(); 260 for (let i = 0; i < dataDirs.length; i++) { 261 let path = GLib.build_filenamev([dataDirs[i], 'gnome-shell', 'theme', name]); 262 let stylesheet = Gio.file_new_for_path(path); 263 if (stylesheet.query_exists(null)) { -
Master
Nit: No braces
-
-
-
Fixed and rebased on master.
-
added 3 commits
-
955df15d...f1b1501f - 2 commits from branch
GNOME:master
- d6d09fd3 - ui: Theme lookup should respect XDG_DATA_DIRS
Toggle commit list -
955df15d...f1b1501f - 2 commits from branch
-
merged
Toggle commit list -
-
-