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 437041 - Add IconTheme support to the Action classes
Add IconTheme support to the Action classes
Status: RESOLVED FIXED
Product: gtkmm
Classification: Bindings
Component: general
2.10.x
Other Linux
: Normal enhancement
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2007-05-09 00:33 UTC by Christian Hammond
Modified: 2008-02-20 13:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Action IconTheme support patch (8.04 KB, patch)
2007-05-09 00:34 UTC, Christian Hammond
none Details | Review
Action IconTheme support patch (11.34 KB, patch)
2007-11-29 00:06 UTC, Christian Hammond
none Details | Review

Description Christian Hammond 2007-05-09 00:33:57 UTC
GTK+ supports setting an icontheme-compliant icon name on an Action (Action, ToggleAction, RadioAction), but gtkmm does not wrap this property. The attached patch adds support for this (as well as wrapping the "visible_overflown" property that was also not wrapped).

It also introduces new Gtk::*Action::create() functions that take an icontheme icon name instead of a stock ID. GTK+ does not provide equivalent functions (even though it should, since it takes stock IDs), but it's something that would be invaluable to VMware Workstation :)
Comment 1 Christian Hammond 2007-05-09 00:34:42 UTC
Created attachment 87848 [details] [review]
Action IconTheme support patch
Comment 2 Murray Cumming 2007-05-14 08:31:56 UTC
I'm a bit worried that this will be an ambiguous method overload:
 static Glib::RefPtr<Action> create(const Glib::ustring& name, const Glib::ustring& label =  Glib::ustring(), const Glib::ustring& tooltip =  Glib::ustring());
   static Glib::RefPtr<Action> create(const Glib::ustring& name, const Gtk::StockID& stock_id, const Glib::ustring& label =  Glib::ustring(), const Glib::ustring& tooltip =  Glib::ustring());
+  static Glib::RefPtr<Action> create(const Glib::ustring& name, const Glib::ustring& icon_name, const Glib::ustring& label, const Glib::ustring& tooltip);
 
Comment 3 Murray Cumming 2007-05-14 08:48:13 UTC
I think, even if this isn't actually ambiguous, it would make things clearer if the create method was renamed to create_*(), with something appropriate. It's a pity that it's too late to rename the existing ones.

I'd also like us (you, please) to take this opportunity to add doxygen documentation for these create methods, and probably the constructors too.

Thanks.
Comment 4 Christian Hammond 2007-05-14 09:19:41 UTC
They're technically not ambiguous due to the lack of default parameters and the number/types of parameters, but they are definitely confusing. I too wondered about just using a create_*(), but wasn't sure if that was alright or if it would be better to use another create() method.

Okay, so given that, what should this be called? create_with_icontheme()? create_with_icon_name()?

I'll do the Doxygen stuff as well. I'll hopefully have a new patch tomorrow or the day after.
Comment 5 Murray Cumming 2007-05-14 09:34:22 UTC
Wouldn't that be create_with_icon_name()?
Comment 6 Jonathon Jongsma 2007-05-14 13:41:19 UTC
One data point: currently we don't have any interfaces named create_with_*, but we do have a few called create_from_* (e.g. Gdk::Pixbuf::create_from_data).  I'm not sure if 'from' works in this case, but it might be something to consider.
Comment 7 Murray Cumming 2007-05-14 14:16:17 UTC
Thanks. I think create_with_*() will be OK here. It makes more sense than create_from_*(). I'm sure we have some other create*() methods.
Comment 8 Murray Cumming 2007-11-09 16:53:33 UTC
Christian, do you feel like finishing this up and committing it?
Comment 9 Christian Hammond 2007-11-09 21:51:43 UTC
Yeah, I have the patch almost finished. Just haven't finished all the docs. I'll look into getting that done maybe today and will put up another diff for review.
Comment 10 Christian Hammond 2007-11-29 00:06:19 UTC
Created attachment 99814 [details] [review]
Action IconTheme support patch

Another iteration of the patch with documentation.
Comment 11 Murray Cumming 2007-11-29 08:01:22 UTC
Excellent. Please commit to svn trunk with a ChangeLog entry. Thanks.

It would be nice if you added @param lines to the documentation.
Comment 12 Jonathon Jongsma 2008-02-19 02:43:25 UTC
Christian: would you like one of us to commit the patch, or would you like to do it?
Comment 13 Murray Cumming 2008-02-20 13:31:21 UTC
Christian seems to have committed this to svn trunk a while ago, but forgot to close the bug:

2007-11-29  Christian Hammond <chipx86@chipx86.com>

	* gtk/src/action.ccg:
	* gtk/src/action.hg:
	* gtk/src/radioaction.ccg:
	* gtk/src/radioaction.hg:
	* gtk/src/toggleaction.ccg:
	* gtk/src/toggleaction.hg: Added create_with_icon_name() functions for
	  the various Action classes, allowing IconTheme icon names to be used
	  instead of stock IDs. This closes bug #437041.