GNOME Bugzilla – Bug 511672
Tooltip for unavailable plugins
Last modified: 2011-02-11 12:24:28 UTC
I'll attach the patch in the next comment
Created attachment 103583 [details] [review] patch I did this patch in the printing branch. Maybe you should change the string if you think it is not appropiate.
Created attachment 103627 [details] [review] patch I forgot a static.
Created attachment 103634 [details] [review] patch Is this what you want?
Created attachment 103639 [details] [review] patch The same but using GError
Created attachment 103738 [details] [review] patch Just a bit change.
Comment on attachment 103738 [details] [review] patch In general I like the patch, especially since at the moment there is no way in the UI to know what's wrong in a plugin that is marked as unavailable in the manager. I wonder if now that we have a way to propagate the error, we shouldn't also show a message dialog the first time a faulty plugin is activated (as far as I recall most of the errors are checked at the first activation). Paolo, Steve, Jessi what do you think? Some comments on the patch itself inline... >- g_warning ("Bad plugin file: %s", file); >+ g_set_error (&info->error, >+ GEDIT_INFO_PLUGIN_ERROR, >+ GEDIT_INFO_PLUGIN_ERROR_AT_LOAD, >+ _("Bad plugin file: %s"), >+ file); The messages to be displayed in UI should be a bit more friendly than what we write in a warning to the console... > gboolean >-gedit_plugin_info_is_available (GeditPluginInfo *info) >+gedit_plugin_info_is_available (GeditPluginInfo *info, >+ GError **error) > { > g_return_val_if_fail (info != NULL, FALSE); > >+ if (info->error && error) >+ *error = g_error_copy ((const GError *)info->error); >+ > return info->available != FALSE; > } I think here you must ensure that if FALSE is returned then an error is always set. > >Index: gedit-plugin-info.h >=================================================================== >--- gedit-plugin-info.h (revision 6112) >+++ gedit-plugin-info.h (working copy) >@@ -41,10 +41,20 @@ > > typedef struct _GeditPluginInfo GeditPluginInfo; > >+#define GEDIT_INFO_PLUGIN_ERROR _gedit_info_plugin_error_quark() >+ >+enum >+{ >+ GEDIT_INFO_PLUGIN_ERROR_AT_LOAD, >+}; >+ Maybe the error codes shuld be a bit more finegrained... >+static gboolean >+query_tooltip_cb (GtkWidget *widget, >+ gint x, >+ gint y, >+ gboolean keyboard_mode, >+ GtkTooltip *tooltip, >+ gpointer useless) >+{ >+ GtkTreeModel *model; >+ GtkTreePath *path; >+ GtkTreeIter iter; >+ gboolean is_row; >+ GeditPluginInfo *info; >+ GError *error = NULL; >+ >+ is_row = gtk_tree_view_get_tooltip_context (GTK_TREE_VIEW (widget), >+ &x, &y, >+ keyboard_mode, >+ &model, >+ &path, >+ &iter); >+ >+ if (is_row) >+ { >+ gtk_tree_model_get (model, &iter, INFO_COLUMN, &info, -1); >+ if (!gedit_plugin_info_is_available (info, &error)) >+ { >+ gtk_tooltip_set_text (tooltip, >+ error->message); >+ I think the tooltip should be a bit more descriptive than just displaying the error message. Somthinhg like: "The plugin foobar could not be loaded. An error occurred while loading the plugin: error->message" >+ g_error_free (error); >+ return TRUE; >+ } >+ } >+ >+ return FALSE; >+} >+ > static void > gedit_plugin_manager_init (GeditPluginManager *pm) > { >@@ -817,6 +854,13 @@ > > pm->priv->tree = gtk_tree_view_new (); > gtk_container_add (GTK_CONTAINER (viewport), pm->priv->tree); >+ /*We have to set the has-tooltip property to TRUE to add Tooltips to rows*/ >+ g_object_set (pm->priv->tree, >+ "has-tooltip", TRUE, >+ NULL); use gtk_widget_set_has_tooltip() instead of manually setting the property.
Created attachment 103778 [details] [review] patch I am not totally sure about the enum. But this patch should fix all issues.
(In reply to comment #7) > Created an attachment (id=103778) [edit] > patch > > I am not totally sure about the enum. But this patch should fix all issues. > My mistake, I uploaded the wrong patch. I'll do it again as soon as possible.
Created attachment 112108 [details] [review] tooltips Right now, this should really fix the bug.
Maybe we should just close this bug? Steve what do you think?
Why not just update the patch to make it work in libpeas?
I think this patch wasn't applied was because it could be too much for the user, we may want to reconsider it, and if you want I can make a patch for peas.
*** Bug 625274 has been marked as a duplicate of this bug. ***
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.