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 726566 - GtkEntryCompletion should emit signal for "no-suggestions" case.
GtkEntryCompletion should emit signal for "no-suggestions" case.
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkEntry
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2014-03-17 17:43 UTC by Saurabh_P
Modified: 2014-06-28 04:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Adding signal to detect "no-suggestions" case (2.11 KB, patch)
2014-03-17 18:34 UTC, Saurabh_P
none Details | Review
Corrected patch (4.48 KB, patch)
2014-06-10 14:54 UTC, Saurabh_P
reviewed Details | Review
Corrected patch (3.11 KB, patch)
2014-06-27 17:08 UTC, Saurabh_P
none Details | Review

Description Saurabh_P 2014-03-17 17:43:27 UTC
I want to implement some task in libgweather and for that I need to detect whether the suggestions list is empty or not. For this and probably in future for some other purpose, adding a signal the emits for "no suggestions" case would be better.
Comment 1 Saurabh_P 2014-03-17 18:34:33 UTC
Created attachment 272194 [details] [review]
Adding signal to detect "no-suggestions" case
Comment 2 Evan Nemerson 2014-03-17 19:07:07 UTC
Instead of special-casing no suggestions, why not emit the signal whenever the number of suggestions changes (and pass the number as an argument)?  Or maybe a read-only property containing the number of suggestions and you just use the notify signal...
Comment 3 Saurabh_P 2014-05-15 19:14:12 UTC
Well no_suggestions is a special case and it'd not be good I guess if we emit signal every time the number of suggestions changes and then we've to check every time that if it is no_suggestions case or not. Plus number of suggestions can be change very rapidly as user changes the input so the callback function will be called too many times. We can implement it but in my opinion we should have this signal separately.
Comment 4 Giovanni Campagna 2014-06-02 10:12:48 UTC
Review of attachment 272194 [details] [review]:

(Informal review)

::: gtk/gtkentrycompletion.c
@@ +310,3 @@
+   *  suggestions)
+   *
+   * Since: 2.4

This should be 3.14

@@ +1303,3 @@
   gtk_tree_model_filter_refilter (completion->priv->filter_model);
 
+  gint rows = gtk_tree_model_iter_n_children (GTK_TREE_MODEL (completion->priv->filter_model), NULL);

This is linear in the number of rows. You can do better by checking the return value of iter_first()

Also, in C89 you can't declare a variable in the middle of code, you need to predeclare it a the top of the block.
Comment 5 Matthias Clasen 2014-06-02 21:18:23 UTC
Giovannis comments are a good starting point. I'll add some more:

From a terminology perspective, the GtkEntryCompletion api is about 'matches', not about suggestions. It would be good to keep with that,and call the signal 'no-matches'.

It would be good to add a vfunc to the GtkEntryCompletion class (and remove one of the remaining padding members).
Comment 6 Saurabh_P 2014-06-10 14:54:42 UTC
Created attachment 278207 [details] [review]
Corrected patch

This is okay I guess.
Comment 7 Matthias Clasen 2014-06-10 21:09:53 UTC
Review of attachment 278207 [details] [review]:

Looks good with the two changes below

::: gtk/gtkentrycompletion.c
@@ +313,3 @@
+   *  suggestions)
+   *
+   * Since: 2.4

This must be 3.14

@@ +1722,3 @@
+    return;
+}
+

Its fine to leave the vfunc as NULL instead of this trivial implementation.
Comment 8 Matthias Clasen 2014-06-10 21:13:22 UTC
Review of attachment 272194 [details] [review]:

::: gtk/gtkentrycompletion.c
@@ +316,3 @@
+                  G_TYPE_FROM_CLASS (klass),
+                  G_SIGNAL_RUN_LAST,
+                  0,

It would be good to add a vfunc to the GtkEntryCompletion class (and remove one of the remaining padding members.
Comment 9 Saurabh_P 2014-06-27 17:08:47 UTC
Created attachment 279424 [details] [review]
Corrected patch
Comment 10 Matthias Clasen 2014-06-28 04:42:39 UTC
committed with a small fixup