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 642554 - GdkEvent not working correctly
GdkEvent not working correctly
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: introspection
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
Depends on:
Blocks: 627482 642922
 
 
Reported: 2011-02-17 10:37 UTC by Ignacio Casal Quinteiro (nacho)
Modified: 2011-02-28 18:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[gi] check to see if object is a member of a union when validating paramaters (2.80 KB, patch)
2011-02-23 19:21 UTC, johnp
accepted-commit_now Details | Review
[gi-overrides] Add event methods to all event union members (3.27 KB, patch)
2011-02-23 23:45 UTC, johnp
none Details | Review
Example of this in action (286 bytes, text/plain)
2011-02-23 23:47 UTC, johnp
  Details
[gi] tests for EventButton override. (1.53 KB, patch)
2011-02-24 18:34 UTC, Laszlo Pandy
none Details | Review

Description Ignacio Casal Quinteiro (nacho) 2011-02-17 10:37:09 UTC
If I have a EventKey and I try to get the state with Gdk.Event.get_state(event) I get this:

  • File "/opt/gnome/lib64/gedit/plugins/snippets/document.py", line 793 in on_view_key_press
    _, state = Gdk.Event.get_state(event)
TypeError: unbound method get_state() must be called with Event instance as first argument (got EventKey instance instead)

My guess is that it is not reconverted to GdkEvent when it is passed as parameter.
Comment 1 johnp 2011-02-17 16:04:27 UTC
what about event.get_state()?  It should be getting the override and should be subclassed as the GdkEvent.  Can you post a small test case.  Thanks
Comment 2 Ignacio Casal Quinteiro (nacho) 2011-02-17 16:23:48 UTC
that doesn't work for some reason now.
Comment 3 Laszlo Pandy 2011-02-17 19:06:44 UTC
The event passed to the callback is not a Gdk.Event, and it's not an override.Gdk.Event either. It is a struct with G_TYPE_VOID.

Gdk.Event is one big union, with a bunch of structs inside. Gdk.Event is the GType of the union, and in this case, Gdk.EventKey is the struct inside of it.

The callback "key-press-event" is annotated as Gdk.EventKey (a type with no GType), as opposed to Gdk.Event. This means that you don't have to do event.key.<attr> to get the attribute name, you can just do event.<attr>. This is how PyGtk worked, and is more typesafe than letting the user access event.button... on something that is really an EventKey.

So changing the type annotations from Gdk.EventKey to Gdk.Event for signals would mostly solve it. To prevent people from having to access the sub-structs explicitly, there is an override which allows event.<attr> to call __getattr__ and get the attribute from the sub-struct. But this breaks because there is a struct called "button" which has a attribute also called "button". So event.button never calls __getattr__.

I'm not sure which is the correct annotation, Gdk.EventKey or Gdk.Event. It's both. Also changing the type annotations in Gtk might also break other bindings who are relying on this type.
Comment 4 johnp 2011-02-23 17:02:44 UTC
So I think I have a solution here.  In our parameter checks if the parameter is a union and the type check fails, check to see if the input type is a member of the union. Now to try and write a patch for that.

Adding to blocker list.
Comment 5 johnp 2011-02-23 19:21:19 UTC
Created attachment 181738 [details] [review]
[gi] check to see if object is a member of a union when validating paramaters

* union members are not subclasses of the union they belong to so if an
   inteface requires you pass a union but you pass one of its members
   there will be a type error
 * this patch checks to see if the type you are passing is a member of the
   union and passes the checks if it is
 * this works in python 3 but in python 2 methods do their own isinstance
   check on the instance parameter (e.g. self) so we need to figure
   out how to override that for union methods (e.g. Gdk.Event.get_state)
Comment 6 johnp 2011-02-23 22:23:28 UTC
So unions are just busted and should never have been exposed in GI and GdkEvent should have been an object instead.  But since the time has passed to fixed this let us commit to only fully supporting GdkEvent unions.

This mean writing overrides for each of the GdkEvent members.  With the patch above here is an example of how we can make this work:

class EventButton(Gdk.EventButton):
    def get_state(self):
        # if python 2 then we have to use the raw function in order to avoid the 
        # instance type check
        return getattr(Gdk.Event.get_state, '__func__', Gdk.Event.get_state)(self)
Comment 7 johnp 2011-02-23 22:56:06 UTC
or better yet

# manually bind GdkEvent members to GdkEvent

modname = globals()['__name__']
module = sys.modules[modname]

# right now we can't get the type_info from the
# field info so manually list the class names
event_member_classes = ['EventButton']

for event_class in event_member_classes:
   override_class = type(event_class, (getattr(Gdk, event_class),))
   # add the event methods
   for method_info in Gdk.Event.__info__.get_methods():
       name = method_info.get_name()
       event_method = getattr(Gdk.Event, name)
       # python2 we need to use the __func__ attr to avoid internal
       # instance checks
       event_method = getattr(event_method, '__func__', event_method)
       setattr(override_class, name, event_method)

   setattr(module, event_class, override(override_class))
   __all__.append(event_class)
Comment 8 johnp 2011-02-23 23:45:31 UTC
Created attachment 181774 [details] [review]
[gi-overrides] Add event methods to all event union members
Comment 9 johnp 2011-02-23 23:47:57 UTC
Created attachment 181775 [details]
Example of this in action
Comment 10 johnp 2011-02-23 23:49:35 UTC
I'm exhausted, someone want to write the tests?  I guess for unions they just have to be a few Gdk override tests since I want to not support unions other than GdkEvents in the future
Comment 11 Laszlo Pandy 2011-02-24 17:34:45 UTC
Comment on attachment 181738 [details] [review]
[gi] check to see if object is a member of a union when validating paramaters

Nice patch, there are just a few style things which should be changed.


>+                gint member_retval;

member_retval is only used inside the for loop and so should be declared in there.

>+                union_info = (GIUnionInfo *) info;

I think generally it is easier to read the code if there aren't multiple pointers pointing to the same object. Since union_info is only used twice, just replace it with "(GIUnionInfo *) info" in those two places, and remove this extra variable.
Comment 12 Laszlo Pandy 2011-02-24 18:34:15 UTC
Created attachment 181857 [details] [review]
[gi] tests for EventButton override.
Comment 13 Laszlo Pandy 2011-02-24 18:39:26 UTC
The tests I posted currently fail. Basically because constructors don't set the event type, and methods such as get_state() always return False unless the type is set to an event type which has a state attribute.
Comment 14 johnp 2011-02-28 18:04:56 UTC
pushed for pre-release.  I'll rewrite the test to use another event that doesn't require user input
Comment 15 johnp 2011-02-28 18:49:00 UTC
Committed a fixed up test which emits the ButtonPress event and does the checks in a event handler.  This fixes the issue of event type not being set by creating a raw event as such:

Gdk.Event.new(Gdk.EventType.BUTTON_PRESS)

and emitting it using button.emit()