After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 511672 - Tooltip for unavailable plugins
Tooltip for unavailable plugins
Status: RESOLVED FIXED
Product: libpeas
Classification: Platform
Component: libpeas-gtk
unspecified
Other Linux
: Normal minor
: ---
Assigned To: libpeas-maint
libpeas-maint
: 625274 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2008-01-23 23:23 UTC by Ignacio Casal Quinteiro (nacho)
Modified: 2011-02-11 12:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (1.94 KB, patch)
2008-01-23 23:24 UTC, Ignacio Casal Quinteiro (nacho)
none Details | Review
patch (1.95 KB, patch)
2008-01-24 11:03 UTC, Ignacio Casal Quinteiro (nacho)
none Details | Review
patch (6.40 KB, patch)
2008-01-24 13:19 UTC, Ignacio Casal Quinteiro (nacho)
none Details | Review
patch (9.03 KB, patch)
2008-01-24 15:51 UTC, Ignacio Casal Quinteiro (nacho)
none Details | Review
patch (9.03 KB, patch)
2008-01-25 17:40 UTC, Ignacio Casal Quinteiro (nacho)
needs-work Details | Review
patch (4.61 KB, patch)
2008-01-26 15:20 UTC, Ignacio Casal Quinteiro (nacho)
none Details | Review
tooltips (8.69 KB, patch)
2008-06-04 08:49 UTC, Ignacio Casal Quinteiro (nacho)
none Details | Review

Description Ignacio Casal Quinteiro (nacho) 2008-01-23 23:23:14 UTC
I'll attach the patch in the next comment
Comment 1 Ignacio Casal Quinteiro (nacho) 2008-01-23 23:24:26 UTC
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.
Comment 2 Ignacio Casal Quinteiro (nacho) 2008-01-24 11:03:44 UTC
Created attachment 103627 [details] [review]
patch

I forgot a static.
Comment 3 Ignacio Casal Quinteiro (nacho) 2008-01-24 13:19:55 UTC
Created attachment 103634 [details] [review]
patch

Is this what you want?
Comment 4 Ignacio Casal Quinteiro (nacho) 2008-01-24 15:51:11 UTC
Created attachment 103639 [details] [review]
patch

The same but using GError
Comment 5 Ignacio Casal Quinteiro (nacho) 2008-01-25 17:40:20 UTC
Created attachment 103738 [details] [review]
patch

Just a bit change.
Comment 6 Paolo Borelli 2008-01-26 14:32:46 UTC
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.
Comment 7 Ignacio Casal Quinteiro (nacho) 2008-01-26 15:20:59 UTC
Created attachment 103778 [details] [review]
patch

I am not totally sure about the enum. But this patch should fix all issues.
Comment 8 Ignacio Casal Quinteiro (nacho) 2008-06-04 08:18:30 UTC
(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.
Comment 9 Ignacio Casal Quinteiro (nacho) 2008-06-04 08:49:21 UTC
Created attachment 112108 [details] [review]
tooltips

Right now, this should really fix the bug.
Comment 10 Ignacio Casal Quinteiro (nacho) 2010-06-24 13:14:23 UTC
Maybe we should just close this bug? Steve what do you think?
Comment 11 Steve Frécinaux 2010-06-25 08:38:00 UTC
Why not just update the patch to make it work in libpeas?
Comment 12 Ignacio Casal Quinteiro (nacho) 2010-06-25 08:44:09 UTC
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.
Comment 13 Steve Frécinaux 2010-07-26 11:12:44 UTC
*** Bug 625274 has been marked as a duplicate of this bug. ***
Comment 14 Garrett Regier 2011-02-11 12:24:28 UTC
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.