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 761446 - ctags: completion results should not include duplicated names
ctags: completion results should not include duplicated names
Status: RESOLVED FIXED
Product: gnome-builder
Classification: Other
Component: plugins
unspecified
Other Linux
: Normal minor
: ---
Assigned To: GNOME Builder Maintainers
GNOME Builder Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-02-02 09:25 UTC by Christian Hergert
Modified: 2016-02-21 23:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch to eliminate duplicates in ctags completions (1.52 KB, patch)
2016-02-14 06:11 UTC, Anoop Chandu
none Details | Review
patch to eliminate duplicates in ctags completions (modified) (1.56 KB, patch)
2016-02-14 14:07 UTC, Anoop Chandu
none Details | Review
GHashtable memory leak is rectified. (1.52 KB, patch)
2016-02-20 20:06 UTC, Anoop Chandu
committed Details | Review

Description Christian Hergert 2016-02-02 09:25:39 UTC
We currently are too liberal in showing duplicated entries in the ctags completion provider. If the word is the same with other completion entries, we should only show duplicate entries if they are of a different type (in case the type icon is useful).

Right now, I think we are showing duplicates if they come from different files. It results in *many* "properties" entries, for example.

This should be patched in the ctags completion provider, where we try to reduce the sorted set of completions.

Our filtering in plugins/ctags/ide-ctags-completion-provider.c:238 sort of expects that the duplicated items would be in the same ctags file. This is not the case, since we often have 10 or more ctags indexes. We need to instead check a global hash table to see if an item exists.

Likely solution:

 - Create a hashtable outside the index search loop (outer loop)
 - Inside the inner loop, hash the name and check if it exists in the hashtable
 - If so, skip the entry
 - Cleanup the hashtable after the search loop

Implementation details:

 - You don't need to copy anything in the hashtable, keep it fast and light
   - g_hash_table_new(g_str_hash, g_str_equal)
 - The index keys will always be valid in this function, no need to track
   pointers to items either. use g_hash_table_contains() instead.
Comment 1 Anoop Chandu 2016-02-14 06:11:12 UTC
Created attachment 321096 [details] [review]
patch to eliminate duplicates in ctags completions

Hashtable is used to store completions .Duplicates in completions are avoided by checking keys for existence in that hashtable.
Comment 2 Christian Hergert 2016-02-14 06:30:32 UTC
Review of attachment 321096 [details] [review]:

Solution looks good, but needs some formatting cleanup

::: plugins/ctags/ide-ctags-completion-provider.c
@@ -204,2 +205,3 @@
   self->results = ide_completion_results_new (self->current_word);
 
+  completions = g_hash_table_new(g_str_hash, g_str_equal);

We use a single space before opening parens in a function call

@@ -236,3 +238,3 @@
           IdeCtagsCompletionItem *item;
 
-          if (ide_str_equal0 (entry->name, last_name))
+          if(g_hash_table_contains(completions,entry->name))

And after "if "

@@ -237,3 +239,3 @@
 
-          if (ide_str_equal0 (entry->name, last_name))
-            continue;
+          if(g_hash_table_contains(completions,entry->name))
+	    	 continue;

Looks like a spurious hard tab got in, 2 space tabs please

@@ +243,1 @@
+	  	   g_hash_table_add(completions,entry->name);

alignment here too
Comment 3 Anoop Chandu 2016-02-14 14:07:11 UTC
Created attachment 321115 [details] [review]
patch to eliminate duplicates in ctags completions (modified)

Attachment "patch to eliminate duplicates in ctags completions" is formatted.
Comment 4 Christian Hergert 2016-02-14 19:15:29 UTC
Review of attachment 321115 [details] [review]:

::: plugins/ctags/ide-ctags-completion-provider.c
@@ -175,2 +175,3 @@
   guint i;
   guint j;
+  GHashTable *completions;

It looks like the hash table is being leaked. Maybe use

g_autoptr(GHashTable) completions = NULL;
Comment 5 Anoop Chandu 2016-02-20 20:06:45 UTC
Created attachment 321741 [details] [review]
GHashtable memory leak is rectified.
Comment 6 Christian Hergert 2016-02-21 23:15:08 UTC
Thanks! Pushed with followup commit to fix passing a const to g_hash_table_add().