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 617551 - Fix passing callbacks as constructor args
Fix passing callbacks as constructor args
Status: RESOLVED FIXED
Product: pygi
Classification: Deprecated
Component: general
unspecified
Other All
: Normal normal
: 0.6
Assigned To: pygi-maint
pygi-maint
Depends on:
Blocks:
 
 
Reported: 2010-05-03 17:14 UTC by Tomeu Vizoso
Modified: 2010-05-05 16:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix passing callbacks as constructor args (3.96 KB, patch)
2010-05-03 17:14 UTC, Tomeu Vizoso
committed Details | Review

Description Tomeu Vizoso 2010-05-03 17:14:03 UTC
.
Comment 1 Tomeu Vizoso 2010-05-03 17:14:05 UTC
Created attachment 160207 [details] [review]
Fix passing callbacks as constructor args
Comment 2 Simon McVittie 2010-05-05 11:06:20 UTC
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.)
Comment 3 Johan (not receiving bugmail) Dahlin 2010-05-05 11:27:27 UTC
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?
Comment 4 Johan (not receiving bugmail) Dahlin 2010-05-05 11:27:28 UTC
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?
Comment 5 Tomeu Vizoso 2010-05-05 11:52:33 UTC
(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.
Comment 6 Tomeu Vizoso 2010-05-05 11:54:16 UTC
(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.
Comment 7 Zach Goldberg 2010-05-05 15:53:46 UTC
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.
Comment 8 Tomeu Vizoso 2010-05-05 16:31:22 UTC
Attachment 160207 [details] pushed as 17fa128 - Fix passing callbacks as constructor args