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 774647 - Wrong function names in Gio::Settings - set_enum(), set_flags() mistyped as get_enum(), get_flags()
Wrong function names in Gio::Settings - set_enum(), set_flags() mistyped as g...
Status: RESOLVED FIXED
Product: glibmm
Classification: Bindings
Component: giomm
2.50.x
Other Linux
: Normal normal
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2016-11-17 20:57 UTC by Daniel Boles
Modified: 2016-11-30 16:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch: fix the wrapped method names (and remove one unnecessary space) (1.39 KB, patch)
2016-11-17 21:02 UTC, Daniel Boles
committed Details | Review

Description Daniel Boles 2016-11-17 20:57:07 UTC
settings.hg:
https://git.gnome.org/browse/glibmm/tree/gio/src/settings.hg#n155
> _WRAP_METHOD(int get_enum(const Glib::ustring& key) const, g_settings_get_enum )
> _WRAP_METHOD(bool get_enum(const Glib::ustring& key, int value), g_settings_set_enum)
> _WRAP_METHOD(guint get_flags(const Glib::ustring& key) const, g_settings_get_flags)
> _WRAP_METHOD(bool get_flags(const Glib::ustring& key, guint value), g_settings_set_flags)

The problem is pretty obvious: the attempts to wrap these set_thing() methods are typo's as get_thing().

This makes these functions unusable from the API - unless the user were to deliberately use the wrong names and thereby render their own code incomprehensible. :P
Comment 1 Daniel Boles 2016-11-17 21:02:57 UTC
Created attachment 340186 [details] [review]
patch: fix the wrapped method names (and remove one unnecessary space)
Comment 2 Marcin Kolny (IRC: loganek) 2016-11-17 22:28:00 UTC
I do agree, this should be fixed.
Comment 3 Murray Cumming 2016-11-18 08:56:05 UTC
Yes. I wonder if we should deprecate the old methods while adding the new ones. Or do we say that they so obviously wrong that nobody could possibly be using them.
Comment 4 Marcin Kolny (IRC: loganek) 2016-11-18 09:11:49 UTC
(In reply to Murray Cumming from comment #3)
> Yes. I wonder if we should deprecate the old methods while adding the new
> ones. Or do we say that they so obviously wrong that nobody could possibly
> be using them.

That's what I thought; probably nobody's used it so far, otherwise I assume he'd report the bug. I think we could safely remove this API.
Comment 5 Daniel Boles 2016-11-18 09:55:48 UTC
I'm also guessing no one used the wrongly named methods, as they would've realised the problem and reported it, but I guess I'm the first pioneer who's using this piece of GSettings in glibmm! Haha.

I think anyone using such opposite method names in live code, with no notice or care for the semantics, is probably overdue for their app to break slightly. ;)
Comment 6 Kjell Ahlstedt 2016-11-24 09:17:12 UTC
The supplied patch shall obviously be pushed to the ABI/API-breaking master
branch.
As for the glibmm-2-50 branch it's safer to add correctly named methods, but
keep and deprecate the incorrectly named ones. But must we be that safe?
Comment 7 Daniel Boles 2016-11-29 20:06:01 UTC
Any more thoughts on whether this will be pushed to master and (hopefully!) 3-22?
Comment 8 Kjell Ahlstedt 2016-11-30 16:36:10 UTC
I have pushed your patch to the master branch, and a similar one to the
glibmm-2-50 branch. The 2-50 patch keeps and deprecates the misnamed methods.
Comment 9 Daniel Boles 2016-11-30 16:40:38 UTC
Thanks a lot!