After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 711520 - GDesktopAppInfo: allow more than one level of legacy folder prefixes
GDesktopAppInfo: allow more than one level of legacy folder prefixes
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2013-11-05 22:36 UTC by Giovanni Campagna
Modified: 2013-11-06 15:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GDesktopAppInfo: allow more than one level of legacy folder prefixes (1.15 KB, patch)
2013-11-05 22:36 UTC, Giovanni Campagna
rejected Details | Review
gdesktopappinfo: keep a list of files in the dirs (14.21 KB, patch)
2013-11-06 14:29 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Giovanni Campagna 2013-11-05 22:36:34 UTC
The menu specification says that each hyphen should be replaced
with a slash, so for a-b-c.desktop we need to try in turn a-b-c.desktop,
a/b-c.desktop, a/b/c.desktop (and not a-b/c.desktop like we did
before).
In particular, this fixes loading wine desktop files, as wine
tries to reproduce a Windows style menu folder.
Comment 1 Giovanni Campagna 2013-11-05 22:36:37 UTC
Created attachment 259050 [details] [review]
GDesktopAppInfo: allow more than one level of legacy folder prefixes
Comment 2 Allison Karlitskaya (desrt) 2013-11-06 01:02:38 UTC
Review of attachment 259050 [details] [review]:

This will conflict with work already on the wip/gdesktopappinfo branch.
Comment 3 Allison Karlitskaya (desrt) 2013-11-06 14:29:01 UTC
Created attachment 259083 [details] [review]
gdesktopappinfo: keep a list of files in the dirs

In each DesktopFileDir, store a list of desktop files for that
directory.  This speeds up opening desktop files by name because we can
skip statting in directories that we know don't have the file and also
speeds up _get_all() because we can avoid enumeration.

This also improves our support for dealing with names like
'kde4/kate.desktop' (equivalent to kde4-kate.desktop) since we find out
about all of these files are the start and don't need to guess about
which '-' to change to a '/'.  It also means that we can easily deal
with more than one level of such prefixes.

We use a file monitor to watch for changes, invalidating our lists when
we notice them.
Comment 4 Lars Karlitski 2013-11-06 15:24:48 UTC
Review of attachment 259083 [details] [review]:

::: gio/gdesktopappinfo.c
@@ +239,3 @@
+  info->desktop_id = g_strdup (app_name);
+
+  g_hash_table_insert (apps, g_strdup (info->filename), info);

doc string says it's using app_name as a key.

@@ +482,3 @@
+
+static void
+desktop_file_dirs_refresh (void)

You got rid of the call to this in g_desktop_app_info_new(). Can we get rid of it in mime_info_cache_init_dir_lists() as well?
Comment 5 Matthias Clasen 2013-11-06 15:38:16 UTC
This prefix nonsense is one of the worst parts of the desktop entry spec.
Comment 6 Allison Karlitskaya (desrt) 2013-11-06 15:38:58 UTC
(In reply to comment #4)
> doc string says it's using app_name as a key.

Good catch.  I had confusion about desktop id vs. filename for a while (and even was replacing the filename in the context above).  I changed that to desktop_id and didn't remember to change the hashtable key at the same time.  Fixed.


> +static void
> +desktop_file_dirs_refresh (void)
> 
> You got rid of the call to this in g_desktop_app_info_new(). Can we get rid of
> it in mime_info_cache_init_dir_lists() as well?

Cleaning up the mime cache machinery will come in a later patch -- for now, all that needs to be done for it is to ensure that the list of directories has been properly populated, and we don't need to hold the lock for that because once established, it will never change.  This is why we refresh() instead of lock() and later unlock().
Comment 7 Allison Karlitskaya (desrt) 2013-11-06 15:56:43 UTC
Attachment 259083 [details] pushed as 86ce3bf - gdesktopappinfo: keep a list of files in the dirs