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 778575 - SimpleAction: Fix strange constructor (takes state arg but claims to create a stateless action)
SimpleAction: Fix strange constructor (takes state arg but claims to create a...
Status: RESOLVED INVALID
Product: glibmm
Classification: Bindings
Component: giomm
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2017-02-13 23:44 UTC by Daniel Boles
Modified: 2017-02-14 08:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
SimpleAction—doc—Erase wrong line & duplicate word (1.04 KB, patch)
2017-02-14 08:18 UTC, Daniel Boles
committed Details | Review

Description Daniel Boles 2017-02-13 23:44:26 UTC
This is probably a leftover from Bug 774444

What's wrong with this picture?

https://developer.gnome.org/glibmm/stable/classGio_1_1SimpleAction.html#a8fc5255c94f94fcca490715993054fee

> Gio::SimpleAction::SimpleAction 	( 	const Glib::ustring&  	name,
> 		const Glib::VariantBase&  	state 
> 	) 		
> 	protected
> 
> Creates a new new stateful action.
> 
> The created action is stateless.
> 
> state is the initial state of the action. All future state values must have the
> same VariantType as the initial state.
> 
> Parameters
>     name	The name of the action.
>     state	The initial state of the action. 

That is: Gio::SimpleAction has this constructor, which despite claiming in the documentation that it creates a stateless Action - nonetheless takes a VariantBase, which it passes as the "state" property of the wrapped GAction.

This presumably creates an Action with a state but with no parameter type for that state, which probably can't be used for any real case - just like the problem with ActionMap::add_with_parameter() fixed (at least for master) in Bug 774444.

This should probably be fixed as ActionMap::add_with_parameter() was, to actually create a working stateful action - i.e. to also take a VariantType indicating the type of the state and pass this to the underlying GAction. This constructor would then be useful, and usable by ActionMap::add_with_parameter()
Comment 1 Daniel Boles 2017-02-13 23:56:54 UTC
Yet again, I didn't notice everything the first time around...

> _WRAP_CTOR(SimpleAction(const Glib::ustring& name, const Glib::VariantType& parameter_type, const Glib::VariantBase& state), g_simple_action_new_stateful)

This already creates the right constructor. So I guess the silly one described above should just be removed. Patch on the way!
Comment 2 Daniel Boles 2017-02-14 00:07:24 UTC
Sorry, ignore me. Removing that ctor and associated create() breaks create_bool(), which clearly depends on it and has been working OK up until now. So somewhere on the mm or C side, someone takes the type from the state, and everything works OK.

So this is a different scenario from Bug 774444, where there was a definite runtime problem if the parameter_type wasn't registered. I'll get some sleep before trying to understand any more code... Apologies for the noise.
Comment 3 Daniel Boles 2017-02-14 08:18:02 UTC
Created attachment 345703 [details] [review]
SimpleAction—doc—Erase wrong line & duplicate word

I pushed a trivial fix to the documentation to remove the incorrect line saying the action is stateless, and fix the "Creates a new new" pasto:

master     : commit e7f82e2dd39d51f3271addb3ad39f78644b22289
glibmm-2-50: commit e8b758a5088c14f989a3f864661c18a6140b3cf0