GNOME Bugzilla – Bug 726566
GtkEntryCompletion should emit signal for "no-suggestions" case.
Last modified: 2014-06-28 04:42:39 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.
Created attachment 272194 [details] [review] Adding signal to detect "no-suggestions" case
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...
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.
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.
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).
Created attachment 278207 [details] [review] Corrected patch This is okay I guess.
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.
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.
Created attachment 279424 [details] [review] Corrected patch
committed with a small fixup