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 655868 - gmenu_tree_get_entry_by_id: New API
gmenu_tree_get_entry_by_id: New API
Status: RESOLVED FIXED
Product: gnome-menus
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-menus dummy account
gnome-menus dummy account
Depends on:
Blocks: 648149
 
 
Reported: 2011-08-03 03:14 UTC by Colin Walters
Modified: 2011-08-06 14:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gmenu_tree_get_entry_by_id: New API (4.81 KB, patch)
2011-08-03 03:14 UTC, Colin Walters
committed Details | Review

Description Colin Walters 2011-08-03 03:14:07 UTC
It's useful for gnome-shell if we have an index by desktop file ID to
the tree entry.  The cost is very small.
Comment 1 Colin Walters 2011-08-03 03:14:10 UTC
Created attachment 193123 [details] [review]
gmenu_tree_get_entry_by_id: New API
Comment 2 Vincent Untz 2011-08-03 06:55:32 UTC
Review of attachment 193123 [details] [review]:

Thanks, it looks good. There are just small details to tweak:

::: libmenu/gmenu-tree.c
@@ +834,3 @@
+  entry = g_hash_table_lookup (tree->entries_by_id, id);
+  if (entry != NULL)
+    gmenu_tree_item_ref (entry);

Nitpicking: can you add an empty line here :-)

@@ +4455,3 @@
+        {
+        case GMENU_TREE_ITEM_INVALID:
+          break;

We don't need this case, it should never happen because of the loop condition.

@@ +4461,3 @@
+
+            item = gmenu_tree_iter_get_entry (iter);
+	    id = gmenu_tree_entry_get_desktop_file_id (item);

There's a mix between tabs and spaces here.

@@ +4474,3 @@
+        default:
+          break;
+        }

nitpick: empty line here

@@ +4500,3 @@
   /* create the menu structure */
+  if (tree->entries_by_id == NULL)
+    tree->entries_by_id = g_hash_table_new (g_str_hash, g_str_equal);

Please do this in gmenu_tree_init() since I don't see any reason for lazy-creation here.

@@ +4543,3 @@
                                                     tree);
     }
+  g_hash_table_remove_all (tree->entries_by_id);

Should be done inside the if block above this. Also, just for consistency with the call to update_entry_index, do it before removing the entries monitor.
Comment 3 Colin Walters 2011-08-06 14:07:14 UTC
Attachment 193123 [details] pushed as f0e4115 - gmenu_tree_get_entry_by_id: New API