GNOME Bugzilla – Bug 437041
Add IconTheme support to the Action classes
Last modified: 2008-02-20 13:31:21 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 :)
Created attachment 87848 [details] [review] Action IconTheme support patch
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);
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.
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.
Wouldn't that be create_with_icon_name()?
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.
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.
Christian, do you feel like finishing this up and committing it?
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.
Created attachment 99814 [details] [review] Action IconTheme support patch Another iteration of the patch with documentation.
Excellent. Please commit to svn trunk with a ChangeLog entry. Thanks. It would be nice if you added @param lines to the documentation.
Christian: would you like one of us to commit the patch, or would you like to do it?
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.