GNOME Bugzilla – Bug 575613
incorrect enum stripping
Last modified: 2015-02-07 16:49:32 UTC
We should be more intelligent and try stripping enums based on a common prefix if they have one, but fall back to the global strip if they don't. Example: typedef enum { META_LAYER_DESKTOP = 0, META_LAYER_BOTTOM = 1, META_LAYER_NORMAL = 2, META_LAYER_TOP = 4, /* Same as DOCK; see EWMH and bug 330717 */ META_LAYER_DOCK = 4, META_LAYER_FULLSCREEN = 5, META_LAYER_FOCUSED_WINDOW = 6, META_LAYER_OVERRIDE_REDIRECT = 7, META_LAYER_LAST = 8 } MetaStackLayer; Members come out like Meta.StackLayer.LAYERTOP, which is pretty busted.
Created attachment 130795 [details] [review] Bug 575613 - Handle enum stripping using common prefix if possible Some enums have members which have a common prefix which doesn't match that of the enum name, but it also longer than the global namespace prefix. Try stripping the common prefix using individual members.
I looked over the gir-repository diffs for this, and it looks generally good to me. Representative changes: diff -ur gir.pre-enum/Avahi-0.6.gir gir/Avahi-0.6.gir --- gir.pre-enum/Avahi-0.6.gir 2009-03-16 16:31:41.000000000 -0400 +++ gir/Avahi-0.6.gir 2009-03-16 18:26:13.000000000 -0400 @@ -14,10 +14,10 @@ <enumeration name="BrowserEvent" c:type="GaBrowserEvent"> <member name="new" value="0" c:identifier="GA_BROWSER_NEW"/> <member name="remove" value="1" c:identifier="GA_BROWSER_REMOVE"/> - <member name="cacheexhausted" + <member name="cache_exhausted" value="2" c:identifier="GA_BROWSER_CACHE_EXHAUSTED"/> - <member name="allfornow" + <member name="all_for_now" value="3" c:identifier="GA_BROWSER_ALL_FOR_NOW"/> <member name="failure" value="4" c:identifier="GA_BROWSER_FAILURE"/> --- gir.pre-enum/Wnck-1.0.tgir 2009-03-16 16:32:09.000000000 -0400 +++ gir/Wnck-1.0.tgir 2009-03-16 18:26:40.000000000 -0400 @@ -1543,14 +1543,14 @@ <member name="splashscreen" value="7"/> </enumeration> <enumeration name="WnckLayoutCorner"> - <member name="wncklayoutcornertopleft" value="0"/> - <member name="wncklayoutcornertopright" value="1"/> - <member name="wncklayoutcornerbottomright" value="2"/> - <member name="wncklayoutcornerbottomleft" value="3"/> + <member name="topleft" value="0"/> + <member name="topright" value="1"/> + <member name="bottomright" value="2"/> + <member name="bottomleft" value="3"/> </enumeration> <enumeration name="WnckLayoutOrientation"> - <member name="wncklayoutorientationhorizontal" value="0"/> - <member name="wncklayoutorientationvertical" value="1"/> + <member name="horizontal" value="0"/> + <member name="vertical" value="1"/> </enumeration> <class name="Workspace" parent="GObject.Object" glib:type-struct="WorkspaceClass" glib:type-name="WnckWorkspace" glib:get-type="wnck_workspace_get_type"> <field name="parent_instance"> --- gir.pre-enum/GstBase-0.10.gir 2009-03-16 16:31:21.000000000 -0400 +++ gir/GstBase-0.10.gir 2009-03-16 18:25:50.000000000 -0400 @@ -1695,7 +1695,7 @@ <member name="started" value="1048576" c:identifier="GST_BASE_SRC_STARTED"/> - <member name="last" + <member name="flag_last" value="4194304" c:identifier="GST_BASE_SRC_FLAG_LAST"/> </bitfield>
Maybe you just want to remove the strip_common_prefix function in utils.py? It doesn't seem to be otherwise used, and has the "remove all _ characters" misfeature. Does your patch work if there is only one child, or do you end up with name='' becaues the prefix is the whole thing? Otherwise it looks good to me. P.S. - GTK+ used to support typedef enum { GTK_TARGET_SAME_APP = 1 << 0, /*< nick=same-app >*/ GTK_TARGET_SAME_WIDGET = 1 << 1, /*< nick=same-widget >*/ } GtkTargetFlags; That one on is silly now because: GTK_TARGET_OTHER_APP = 1 << 2, /*< nick=other-app >*/ GTK_TARGET_OTHER_WIDGET = 1 << 3 /*< nick=other-widget >*/ were added, though the nick= annotations weren't removed. And I don't see any other legitimate uses in pango/gtk+. So, probably best to wait for a feature request before worrying about how to do such overrides :-)
If it's a registered enum, we already use the nick for the introspection name. Though this actually brings up a different bug; since convention is "-" in enum nicks, should we be replacing those with "_" in introspection? My feeling is yes, since most bindings are already doing this now.
Created attachment 130829 [details] [review] Bug 575613 - Enum stripping with common prefix, also use "_" consistently Some enums have members which have a common prefix which doesn't match that of the enum name, but it also longer than the global namespace prefix. Instead, try stripping the common prefix first, and only if that fails fall back to the global strip. Also, for glib-registered enums we were using the nick, which typically has "-" as a separator. Replace that with "_" for consistency between unregistered enums and registered. utils.py:strip_common_prefix is now unused, delete.
Thanks for the review. In the new patch we explicitly handle the single-member case you pointed out (with test), deleted the unused function, and I also we ahead and did the change described in comment #4.
Went ahead and committed.
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]