GNOME Bugzilla – Bug 667861
Anjuta should automatically create members for ui widgets.
Last modified: 2012-01-18 20:11:13 UTC
Created attachment 205180 [details] [review] Automatically creates private members for widgets on double click in glade inspector. The attached patch adds a new feature to anjuta. Now, the user only has to double click a widget in glade inspector and code for member declaration and initialization is automatically created for the widget. Please review it.
Review of attachment 205180 [details] [review]: There are some general points: * This features needs documentation, otherwise it is not obvious. * At least all the C project types (and preferably all C++) project type should include the necessary comments. Vala support is probably easy to add. * The comments should have some kind of mark that they belong to the Anjuta code generation and aren't user comments so people don't delete them (or know what they are doing). * How do you know in which file/class to add the widget? ::: libanjuta/interfaces/libanjuta.idl @@ +1282,3 @@ + * + * This signal is emitted when code is added inside the editor. + * The newly added code is @code which has been inserted at @position. The documentation is wrong. ::: plugins/glade/plugin.c @@ +347,3 @@ + docman = anjuta_shell_get_interface (ANJUTA_PLUGIN (plugin)->shell, + IAnjutaDocumentManager, NULL); + if(!docman) return; Please indent the return statement to the next line. @@ +349,3 @@ + if(!docman) return; + + doc = ianjuta_document_manager_get_current_document (docman, NULL); I think it would be more useful to query the current editor from the GladePlugin argument. ::: plugins/language-support-cpp-java/plugin.c @@ +857,3 @@ +on_glade_member_add (IAnjutaEditor *editor, gchar *widget_name, CppJavaPlugin *lang_plugin) +{ + printf("Signal received in lang-support!\n"); Please convert those to DEBUG_PRINT() or remove them. @@ +894,3 @@ + + printf("Connecting glade-member-add from editor %p to lang-support.\n", lang_plugin->current_editor); + if (!g_signal_handler_find (lang_plugin->current_editor, Why is this "if" needed? The signal handler shouldn't be connected when this is called.
(In reply to comment #1) > * How do you know in which file/class to add the widget? Code for the widget will be generated in the current editor. As discussed on IRC, this approach is acceptable. > @@ +349,3 @@ > + if(!docman) return; > + > + doc = ianjuta_document_manager_get_current_document (docman, NULL); > > I think it would be more useful to query the current editor from the > GladePlugin argument. AFAICS, priv->editor is a pointer to a GladeEditor, not the "current editor". > @@ +894,3 @@ > + > + printf("Connecting glade-member-add from editor %p to > lang-support.\n", lang_plugin->current_editor); > + if (!g_signal_handler_find (lang_plugin->current_editor, > > Why is this "if" needed? The signal handler shouldn't be connected when this is > called. I forgot to disconnect the signal on uninstall_support. Should not be needed now. All other points should be fixed in next iteration.
Also, what do you suggest for the markers? I was thinking something about these lines: /* Marker for Member Widgets */ /* Marker for Member Widgets - DO NOT REMOVE */ /* Anjuta Marker for Member Widgets */ /* Anjuta Marker for Member Widgets - DO NOT REMOVE */ And by documentation you mean something like these: http://git.gnome.org/browse/anjuta/tree/manuals/anjuta-manual/C/anjuta-glade-signals.page http://git.gnome.org/browse/anjuta/tree/manuals/anjuta-manual/C/anjuta-glade-start.page ?
Yep, sorry it's a bit incomplete at the moment. For the marker I think I would prefer /* ANJUTA: Widgets - DO NOT REMOVE */
Created attachment 205353 [details] [review] Automatically creates private members for widgets on double click in glade inspector. The attached fixes points complained by jhs. Please review it.
Review of attachment 205353 [details] [review]: Overall this looks very good - thanks, especially for the docs. Sorry for some minor nitpicking but it's better to fix it know and to have a clean patch. Great work! I look forward to see this for other languages (C++, Vala, Python), too! ::: manuals/anjuta-manual/C/anjuta-glade-start.page @@ +67,3 @@ + current project. By default, the file that will hold the code for the + widgets is the same one where callbacks will be created (eg: + <file>gtk-foobar.c</file>). Both files can be easily found in the project file list "be found" - sorry minor grammer nitpicking and I think the file is called application.c or so in the standard project type so use that as an example, please. @@ +73,3 @@ + <p>Once the glade plug-in is running and the file that will hold the code + is being viewed, simply double click a widget in the glade inspector. + The file being viewed will then be scanned for some marker comments You should list the marker comments that are used here. ::: plugins/project-wizard/templates/gtk/src/main.c @@ +24,3 @@ +} __priv; + +static struct _priv* priv = &__priv; Please write it this way: typedef struct _Private Private; struct Private { /* Marker... */ }; static Priv* priv;
Created attachment 205434 [details] [review] Automatically creates private members for widgets on double click in glade inspector. The attached patch fixes complaints from previous comment. Please review it.
Created attachment 205515 [details] [review] Automatically creates private members for widgets on double click in glade inspector. The attached patch fixes a memory leak introduced in the las version. Please review it.
Review of attachment 205515 [details] [review]: See comment with the filename, otherwise, looks ok ::: manuals/anjuta-manual/C/anjuta-glade-start.page @@ +67,3 @@ + current project. By default, the file that will hold the code for the + widgets is the same one where callbacks will be created (eg: + <file>gtk-foobar.c</file>). Both files can easily be found in the project file list See the comment on my last review.
Created attachment 205578 [details] [review] Automatically creates private members for widgets on double click in glade inspector. The attached patch fixes last remaining details. Please review it.
Review of attachment 205578 [details] [review]: ok