GNOME Bugzilla – Bug 480026
Metacity should use XDG_DATA_{DIRS,HOME} to search for themes
Last modified: 2009-10-13 17:33:16 UTC
It would be nice if metacity could look in XDG_DATA_{DIRS,HOME} to find themes, as well as ~/.themes/. I believe GTK+ already does this for its themes as well. GLIB also provides API for getting the contents of the variables. Should be a trivial patch.
<i>Should be a trivial patch.</i> I'll mark it as gnome-love, then, but refer people to you if they have questions. :-)
where is the code located that handles this? is it part of metacity or is it in the gnome appearance capplet? I'll fix it if I can find it. (at least I'll try to fix it.)
Both would need to be updated. I believe the themes capplet already handles XDG_DATA_DIRS though. If not, it's also a trivial patch for it as well. Should be in the theme handling code there as well. I don't know exactly where the code to be modified is in either case.
Here's my plan to fix this: 1) Add code to theme-parser.c to look for themes in XDG_DATA_DIRS & XDG_DATA_HOME after checking in METACITY_DATADIR. 2) Edit makefile to define XDG_DATA_DIRS & XDG_DATA_HOME in the same manner that METACITY_DATADIR is defined. Rodney, was right, the themes capplet already handles XDG_DATA_DIRS. Is this approach correct?
No. $XDG_DATA_{DIRS,HOME} are environment variables. You shouldn't put them in Makefile.am as defines. You need to use the glib API to get their values, and then check them in the appropriate order. Check $XDG_DATA_HOME/themes, then ~/.themes, then METACITY_DATADIR, and then loop through each of the values for $XDG_DATA_DIRS and check in $XDG_DATA_DIRS/themes. You might want to check METACITY_DATADIR after $XDG_DATA_DIRS even. I'll gladly implement the patch if you would prefer. I'm just busy with other things right now, and filed this bug from an IRC discussion the other night. :) Give me another day or two, and I can probably cook up a quick patch.
I'd really like the opportunity to work on a patch. So if you wouldn't mind, could you help me write the patch instead of doing it on your own? Thank you for your help so far. If I'm on the right track, after following what you said, I should use the glib function g_get_system_data_dirs to get the $XDG_DATA_DIRS environment variable, correct?
Created attachment 96440 [details] [review] proposed patch Patch is tested and works on my machine. Ubuntu Gutsy Beta.
Will test tomorrow.
Uh, you're much too late for 2.21.2. ;-)
Benjamin: Do you think we should break once and for all at the bottom of the for loop, if theme_file is not NULL, rather than checking whether theme_full *is* null at the top of the loop every time thereafter? Otherwise it seems to be fine (though I haven't run it yet).
Thomas, thank you for taking a look at my patch. Of course once the theme is found Metacity should stop looking for it! I'll update the patch to reflect the change and upload it tomorrow.
Created attachment 99127 [details] [review] changed per Thomas's comment Will this work? Did I understand what you mean?
There is API in GLib now to get the data dirs I believe, so we shouldn't need to hand-parse the env variables in metacity.
yes, indeed: g_get_system_data_dirs() -- I hadn't seen that before. Benjamin, would you consider writing the patch in terms of that? Sorry for the inconvenience.
I didn't know about these functions. Havoc, thank you for pointing this out. I would be happy to rewrite the patch using these, just give me a day or two.
http://standards.freedesktop.org/basedir-spec/latest/ should be linked somewhere in this bug report, so here we are linking it-- although it's really now glib's problem and not ours to comply, of course.
Created attachment 99222 [details] [review] uses g_get_system_dirs() This should work.
Lovely work! Thank you! It appears to work fine; I've committed it: http://svn.gnome.org/viewvc/metacity?view=revision&revision=3411 but if all your work is this good, I'll have no hesitation in recommending you for svn access.