[Xcb] [PATCH 2/2: xpyb] Fix length field handling, again.

Alex Plotnick shrike at netaxs.com
Fri Mar 23 07:59:23 PDT 2012


At or around Fri, 23 Mar 2012 07:32:56 PDT, Jamey Sharp said:

> Are you guaranteed that expr.parent and expr.parent.parents both
> exist? The check for AttributeError before can only have been guarding
> those field accesses, since hasattr won't throw that, right?

Like I said in the commentary, this whole bit of logic is one big special
case. So the answer is: I'm not yet sure if it's always guaranteed that
expr.parent.parents is always a valid reference, but since it works, I'm
more inclined to leave it as-is and deal with an error that crops up
later than to pre-emptively catch all errors and generate incorrect code
(which is what happened with c5ff7fb).

If I have time, I'll see if I can replace this whole function with
something that actually does the right thing in all possible circumstances.
It shouldn't be too hard (famous last words, right?).

> I'm not an xpyb user so I should probably stay out of the question,
> but I don't see why you shouldn't use isinstance here. It seems more
> clear to me, even if it's officially less Pythonic.

I don't think it's less Pythonic, and I completely agree (and noted it
in my commentary on the patch). All of xcbgen is extremely hostile towards
isinstance (see Type.__init__); if it were up to me, I'd rip all that out
and replace it with isinstance calls. But in the interest of minimal changes,
I left it as-is.

	-- Alex


More information about the Xcb mailing list