Created attachment 160207 [details] [review]
Fix passing callbacks as constructor args

Would it make sense to combine is_method and is_constructor into an enum for "callable type"?

(Other potentially-interesting values other than function and method: static method, class method, generator, or even class.)

Review of attachment 160207 [details] [review]:

Looks good to me though.

::: gi/pygi-callbacks.c
@@ +179,2 @@
     /* if its a method then we need to skip over 'self' */
+    if (is_method || is_constructor)

Weird. constructors doesn't have a self argument, do they?

Review of attachment 160207 [details] [review]:

Looks good to me though.

::: gi/pygi-callbacks.c
@@ +179,2 @@
     /* if its a method then we need to skip over 'self' */
+    if (is_method || is_constructor)

Weird. constructors doesn't have a self argument, do they?

(In reply to comment #2)
> Would it make sense to combine is_method and is_constructor into an enum for
> "callable type"?
> 
> (Other potentially-interesting values other than function and method: static
> method, class method, generator, or even class.)

Could be, but right now I'm having trouble anticipating the different ways in which methods should be handled based on their types. I'm sure we are still missing some less-common cases, so we'll have chances to revisit it soon.

(In reply to comment #4)
> Review of attachment 160207 [details] [review]:
> 
> Looks good to me though.
> 
> ::: gi/pygi-callbacks.c
> @@ +179,2 @@
>      /* if its a method then we need to skip over 'self' */
> +    if (is_method || is_constructor)
> 
> Weird. constructors doesn't have a self argument, do they?

Well, not in the gobject side but in python they have the class as the first arg. That's why we don't care about it when we look for the callback args, but need to take it into account when finding the respective python args.

Review of attachment 160207 [details] [review]:

Patch looks alright to me.  I sadly cannot at the moment confirm that before the patch this test fails and after the patch the test works because I dont have a pygi install on my work machine.  If that is the case then I am happy with this.

Attachment 160207 [details] pushed as 17fa128 - Fix passing callbacks as constructor args