On Tue, Nov 10, 2009 at 8:42 AM, Antoine Martin <[email protected]> wrote: > Hi Nathaniel, > > I was looking at the changes whilst rebasing my patches on mainline and > I've got a few questions: > > 1) > def draw(self, x, y, width, height, rgb_data): > assert len(rgb_data) == width * height * 3 > (my_width, my_height) = self.window.get_size() > gc = self._backing.new_gc() > self._backing.draw_rgb_image(gc, x, y, width, height, > gtk.gdk.RGB_DITHER_NONE, rgb_data) > self.window.invalidate_rect(gtk.gdk.Rectangle(x, y, width, height), > False) > > my_width and my_height aren't used, lint told me ;)
Good catch, fixed. > 2) As mentionned in the other email, can you please merge this patch: > http://xpra.devloop.org.uk/patches/v0.0.8pre/mswindows-move-eventcode.patch > This really seems like a reasonable thing to do and would massively > reduce the amount of rebasing I have to do to keep win32 support in sync. > Please, please, please. See my reply to the other email (when I finish writing it :-)). > 3) xvfb switch: > xvfb = subprocess.Popen(["Xvfb-for-Xpra-%s" % display_name, > display_name, > "-auth", xauthority, > "+extension", "Composite", > # Biggest easily available monitors are > # 1920x1200. This is 1920*2 x 1920: > "-screen", "0", "3840x1920x24+32", > "-once"], > executable="Xvfb") > > Did you test this? > executable="Xvfb" should be opts.xvfb surely? > Also AFAIK, this does not allow you to specify arguments to Xvfb (or > Xvnc), like: --xvfb="Xvnc --PasswordFile=blah" Err, doh. Yeah, apparently half the patch got lost somewhere... maybe I somehow reverted xpra/scripts/server.py before committing? Anyway, now committed and tested. And I'm a bit confused about the complaint about passing arguments, because one of the things I disliked about your patch is that it didn't allow passing arguments :-). But yes, the version in mainline supports passing arguments. > 4) Still some unused imports, ie: xpra/scripts/server.py You mean the 'import time'? Fixed. (And I swear I did this before too... oh well.) > And "from wimpiggy.error import *" is making lint shout at me. :( Okay, like I said I don't think it matters much at this point, so send a patch :-). (If I did it I'd probably be too lazy to look up exactly which files reference 'trap', which reference 'XError', and which reference both, and then you'd just end up sending me a patch complaining about unused variables anyway, so this way seems simpler.) -- Nathaniel _______________________________________________ Parti-discuss mailing list [email protected] http://lists.partiwm.org/cgi-bin/mailman/listinfo/parti-discuss
