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 621857 - Configurable inteface is too specific (to gedit/totem)
Configurable inteface is too specific (to gedit/totem)
Status: RESOLVED FIXED
Product: libpeas
Classification: Platform
Component: general
git master
Other All
: Normal normal
: ---
Assigned To: libpeas-maint
libpeas-maint
Depends on:
Blocks:
 
 
Reported: 2010-06-17 07:31 UTC by Johannes Schmid
Modified: 2010-07-01 15:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PeasUIConfigurable] Rename create_configure_dialog() to _widget() (5.81 KB, patch)
2010-07-01 11:43 UTC, Steve Frécinaux
committed Details | Review
[PeasUIPluginManager] Create a dialog to host the configure widget. (4.95 KB, patch)
2010-07-01 12:04 UTC, Steve Frécinaux
committed Details | Review

Description Johannes Schmid 2010-06-17 07:31:42 UTC
The configurable interface assumes that each plugin opens a dialog for configuration which is not true for applications that use the "everything is a plugin" approach like anjuta. It would be better if configurable could simply return a GtkWidget* and let the application decide how to handle it.

Same applies for the "About" and "Configure" buttons in the plugin-manager. There should at least be an option to hide this buttons when the application decides to show this information in a different way.
Comment 1 Steve Frécinaux 2010-06-17 09:36:39 UTC
I agree with this one. The current libpeasui is mostly a copy-paste for now and it should be made more flexible for such needs.
Comment 2 Philip Withnall 2010-06-27 12:04:06 UTC
The current API is ugly, too. Is there any particular reason the widget is returned via an out parameter? Why not just return GtkWidget* and return NULL on failure? The current API is nasty for bindings.
Comment 3 Steve Frécinaux 2010-06-30 18:56:30 UTC
See bug 623173 for the ugly API.
Comment 4 Johannes Schmid 2010-07-01 11:03:57 UTC
What bug 623172 doesn't solve is, that the returned GtkWidget is assumed to be a dialog while I would assume that it is a GtkWidget that can be put anywhere the applications decides too.

As the GNOME policy is to have all preferences apply instantly, there is no reason why the plugin should need to care about dialog actions, etc.

I would propose to have
GtkWidget  *peas_ui_configurable_create_configure_widget (PeasUIConfigurable  *configurable);

and to have the application use it like they want to. Of course, for its plugins, the application may require that this widget be a GtkDialog.

I am willing to provide a patch for this and for making the plugin-manager a single tree view widget but I want to know first that this would be accepted. I don't want to patch gedit and totem though ;)
Comment 5 Steve Frécinaux 2010-07-01 11:34:56 UTC
(In reply to comment #4)
> What bug 623172 doesn't solve is, that the returned GtkWidget is assumed to be
> a dialog while I would assume that it is a GtkWidget that can be put anywhere
> the applications decides too.

Sure, but that will be addressed in a separate patch.

> and to have the application use it like they want to. Of course, for its
> plugins, the application may require that this widget be a GtkDialog.

Actually, I plan to make the plugin manager widget automatically wrap the widget in a GtkDialog.

> I am willing to provide a patch for this and for making the plugin-manager a
> single tree view widget but I want to know first that this would be accepted. I
> don't want to patch gedit and totem though ;)

I'd be more willing to split out the tree view from the plugin manager as a separate widget but let the opportunity to applications that don't care to use the builtin manager widget.

We can even consider adding a second style of plugin manager widget if someone cares. I know for instance that totem and rhythmbox used to have the plugin info beside the tree view. The currently used dialog was only used by gedit afaik...

That said, patches are of course welcome :-)
Comment 6 Steve Frécinaux 2010-07-01 11:43:14 UTC
Created attachment 165019 [details] [review]
[PeasUIConfigurable] Rename create_configure_dialog() to _widget()

We want to return a widget instead of a dialog, so plugin managers can
do whatever they want with the widget instead of just being able to
display a configuration dialog.
Comment 7 Ignacio Casal Quinteiro (nacho) 2010-07-01 11:43:53 UTC
Well I think would be better having a consensus in relation to the widget that should use all the apps. About that this widget is only used by gedit is not true, eog, vinagre and eog also uses it.
Comment 8 Johannes Schmid 2010-07-01 11:54:12 UTC
As already explained a bit in the initial bug comment I don't think that there will be a plugin-manager that will fit for the both extremes of plugin component architecture.
 * Everything is a plugin (anjuta, ...)
 * App can be extended with plugins (gedit, totem, etc.)

Anjuta doesn't want to look like a set of plugins of course but instead provide a sane interface also for preferences. That means that plugins shouldn't have their own dialogs for configuration. That has been addressed already by Steve's patch in comment #6 (thanks!).

The other thing is that anjuta wants to embed the plugin-manager in its preferences UI so it would like to have just the content of that dialog. Maybe we could keep the plugin-manager as a widget (which can be added to an appropriate dialog by the application) while giving the option to hide the additional buttons. Splitting out the tree view can be another solution though. Two widgets sounds like a maintenance nightmare.

For the visual approach, I would prefer it to look like it is currently done in most applications where the tree view contains the name and the short description (in smaller font) together with an icon.
Comment 9 Steve Frécinaux 2010-07-01 12:04:20 UTC
Created attachment 165020 [details] [review]
[PeasUIPluginManager] Create a dialog to host the configure widget.

With the PeasUIConfigurable API changed to return a widget instead of a
dialog, we need to create the dialog to host that widget ourselves.

Also we need to ensure plugins don't return a dialog.
Comment 10 Steve Frécinaux 2010-07-01 12:14:29 UTC
(In reply to comment #8)
> The other thing is that anjuta wants to embed the plugin-manager in its
> preferences UI so it would like to have just the content of that dialog. Maybe
> we could keep the plugin-manager as a widget (which can be added to an
> appropriate dialog by the application) while giving the option to hide the
> additional buttons. Splitting out the tree view can be another solution though.

I think I'd prefer two simpler widgets (full-fledged manager and simpler treeview) to adding options to the manager widget and exposing additional stuff.

> Two widgets sounds like a maintenance nightmare.

Well, not as long as those widgets just consist in packing two other widgets together... But I'd prefer a consensus among the plugins users for sure.

> For the visual approach, I would prefer it to look like it is currently done in
> most applications where the tree view contains the name and the short
> description (in smaller font) together with an icon.

This is what it looks like right now isn't it?

Besides, I think the rework of the plugin manager / plugin tree view should be discussed in another bug...
Comment 11 Johannes Schmid 2010-07-01 12:55:54 UTC
> I think I'd prefer two simpler widgets (full-fledged manager and simpler
> treeview) to adding options to the manager widget and exposing additional
> stuff.
> 
> > Two widgets sounds like a maintenance nightmare.
> 
> Well, not as long as those widgets just consist in packing two other widgets
> together... But I'd prefer a consensus among the plugins users for sure.

Yeah, that what I meant. Probably I should have talked about "independent widgets". Stacked widgets would be ok of course.
 

> This is what it looks like right now isn't it?

Yes, I think so. And I don't think anything needs to change. I was a bit referring to nacho's comment that all apps should use the same visual approach.
Comment 12 Steve Frécinaux 2010-07-01 15:28:47 UTC
Comment on attachment 165019 [details] [review]
[PeasUIConfigurable] Rename create_configure_dialog() to _widget()

Attachment 165019 [details] pushed as 23c78c4 - [PeasUIConfigurable] Rename create_configure_dialog() to _widget()
Comment 13 Steve Frécinaux 2010-07-01 15:29:14 UTC
Attachment 165020 [details] pushed as 47c6e95 - [PeasUIPluginManager] Create a dialog to host the configure widget.