GNOME Bugzilla – Bug 635913
Port dictionary plugin to libpeas
Last modified: 2010-11-28 22:46:53 UTC
I attached the patches which enable to use the dictionary plugin using the libpeas GObject system. This is my task for Google Code-In 2010. [1] http://www.google-melange.com/gci/task/show/google/gci2010/gnome/t129020287448
Created attachment 175343 [details] [review] 0001-fetched-gdict-sidebar.-ch-from-gnome-utils-master
Created attachment 175344 [details] [review] 0002-Port-dictionary-plugin-to-libpeas.
Created attachment 175345 [details] [review] 0003-Update-and-indent-source-and-fix-corrupt-Makefile
Created attachment 175346 [details] [review] 0004-Update-identifier-in-dictionary-plugin-desktop-file
Created attachment 175349 [details] [review] de-activate configuration dialog
Review of attachment 175343 [details] [review]: Comments inline ::: plugins/dictionary/gtr-gdict-sidebar.c @@ +77,3 @@ G_DEFINE_TYPE (GdictSidebar, gdict_sidebar, GTK_TYPE_VBOX); +SidebarPage * change this back to static @@ +93,3 @@ } +void same @@ +216,2 @@ static void +gdict_sidebar_close_clicked_cb (GtkWidget * widget, gpointer user_data) remove this @@ +348,3 @@ gtk_widget_show (select_button); + close_button = gtk_button_new (); remove all this too about the close button
Review of attachment 175344 [details] [review]: comments inline. ::: plugins/dictionary/gtr-dictionary-plugin.c @@ +37,2 @@ +struct _GtrDictPluginPrivate + { wrong indentation here. it shouldn't have any space @@ +37,3 @@ +struct _GtrDictPluginPrivate + { + GSettings *settings; I think you can remove the settings, afaik you are not using them for anything. @@ +153,1 @@ + if (priv->settings) remove the settings. @@ +199,3 @@ + GTR_TYPE_WINDOW_ACTIVATABLE, + GTR_TYPE_DICT_PLUGIN); + peas_object_module_register_extension_type (module, you don't need this.
Review of attachment 175345 [details] [review]: Comments inline ::: plugins/dictionary/Makefile.am @@ +5,3 @@ -I$(top_srcdir) \ -I$(top_srcdir)/src \ + $(GTRANSLATOR_CFLAGS) \ indent this correctly the \ should be in the same place alltogheter ::: plugins/dictionary/gtr-dictionary-plugin.h @@ +34,3 @@ #define GTR_IS_DICT_PLUGIN(o) (G_TYPE_CHECK_INSTANCE_TYPE ((o), GTR_TYPE_Dict_PLUGIN)) #define GTR_IS_DICT_PLUGIN_CLASS(k) (G_TYPE_CHECK_CLASS_TYPE ((k), GTR_TYPE_Dict_PLUGIN)) #define GTR_Dict_PLUGIN_GET_CLASS(o) (G_TYPE_INSTANCE_GET_CLASS ((o), GTR_TYPE_Dict_PLUGIN, GtrDictPluginClass)) this should be DICT instead of Dict @@ +35,3 @@ #define GTR_IS_DICT_PLUGIN_CLASS(k) (G_TYPE_CHECK_CLASS_TYPE ((k), GTR_TYPE_Dict_PLUGIN)) #define GTR_Dict_PLUGIN_GET_CLASS(o) (G_TYPE_INSTANCE_GET_CLASS ((o), GTR_TYPE_Dict_PLUGIN, GtrDictPluginClass)) +typedef struct _GtrDictPlugin GtrDictPlugin; here is missing one newline @@ +53,3 @@ }; +GType put this in teh same line, the indent script is good to run once if the code is all screwed but then it is better to fix some things manually.
Review of attachment 175346 [details] [review]: looks good
Review of attachment 175349 [details] [review]: comments inline ::: plugins/dictionary/gtr-dictionary-plugin.c @@ +52,3 @@ + iface); + +G_DEFINE_DYNAMIC_TYPE_EXTENDED (GtrDictPlugin, please reindent this correctly
> @@ +199,3 @@ > + GTR_TYPE_WINDOW_ACTIVATABLE, > + GTR_TYPE_DICT_PLUGIN); > + peas_object_module_register_extension_type (module, > > you don't need this. If I remove this the dictionary sidebar won't show up anymore. So I'll leave this until you show me that I'm wrong.
Created attachment 175383 [details] [review] Port dictionary plugin to libpeas
Pushed the patch. Thanks a lot.