GNOME Bugzilla – Bug 667570
Implement callback name suggestion on glade-signal-editor
Last modified: 2012-03-19 22:38:56 UTC
Created attachment 204870 [details] [review] Patch implementing callback name suggestion on glade-signal-editor The attached implements callback name suggestion on glade-signal-editor for glade 3.10. Not that it is not a port from the one available in glade 3.8. Please review it.
Created attachment 209539 [details] [review] Signal name completion. The attached patch is a partial port of the callback name completion feature from glade 3.8. I think it is good enough to be committed as is. Please review it.
Created attachment 209596 [details] [review] Signal name completion. The attached patch is a complete port of the callback name completion feature from glade 3.8. Please review it.
Created attachment 209602 [details] [review] Signal name completion. The attached patch fixes a memory leak in the previous one. Please review it.
Patch looks good functionally speaking for couple of minor issues. First: Please add strings to the call to g_param_spec_object() for the property nick and description. I.e. "Handler Completion" for the nick and "The completion to use when typing in signal handler names". Otherwise this will have bad results in building documentation. Second: Along the same lines, you're missing this text just before the added api in glade-signal-editor.c: /** * glade_signal_editor_set_completion_store: * @self: A #GladeSignalEditor * @store: (allow-none): The #GtkListStore to set, or %NULL * * Sets @store as the list store to use for completions when * completing the handler. * * <note><para>The list store will be modified by Glade during * usage, additional handler names will be added. The passed liststore * is expected to have a single %G_TYPE_STRING column.</note></para> */ This will show up in the docs as well as aid with generation of the introspection data for libgladeui. Note, perhaps this is not the best api, it seems awkward to pass a list store here and not a GtkTreeModel, however since Glade accesses and modifies it; it must be a strict list store type... I'm not entirely happy with the api for IDEs to provide signal handler suggestions, perhaps we should temporarily take the previous patch and not add API until we have a better api to do this more explicitly... then again... it's of little importance and the proposed api will work, so for the record I'm not against applying this.
Created attachment 209830 [details] [review] Signal name completion. The attached patch fixes documentation issues from last comment. Please consider it for inclusion.
Adding a new function would break API freeze, besides I really do not like to exposean implementation detail in the API, so I think I might rework the patch to add a new signal callback_suggestions that would be fired when needed and as a default implementation we will complete it with what this patch currently implements. But this leave us the possibility to add new handlers that could for example be defined by the user in the config file etc
Note that exposing this api was originally to allow IDEs to completely override the completions suggested by Glade. This makes sense as specially if the IDE is currently editing code for a language that is not C. The idea of saving a history of suggested names in the session file is not a bad one per-se, but should be implemented as a part of Glade's frontend code so as not to interfere with Anjuta or other frontends. Glade's frontend could build the list of suggested names from the config file and override the signal suggestions in the same way that Anjuta does (whatever api that we finally use).
Implemented GladeSignalEditor::callback-suggestions signal