[webkit-reviews] review denied: [Bug 100062] nrwt: truncate meter lines properly on windows : [Attachment 170046] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 100062: nrwt: truncate meter lines properly on windows
https://bugs.webkit.org/show_bug.cgi?id=100062

Attachment 170046: Patch
https://bugs.webkit.org/attachment.cgi?id=170046&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=170046&action=review


Putting the method in platforminfo seems fine.	I would probably pass in the
number of columns rather than the whole platforminfo object (Makes it easier to
test? I think I read this in a TotT.).

> Tools/Scripts/webkitpy/common/system/platforminfo.py:92
> +	       if self.is_win():

Looks like cygwin is also considered win, but I think we want cygwin to take
the 'else' code path.

> Tools/Scripts/webkitpy/common/system/platforminfo.py:95
> +		   h = windll.kernel32.GetStdHandle(-12)  # handle for stderr

Nit: h -> handle

> Tools/Scripts/webkitpy/common/system/platforminfo.py:96
> +		   csbi = create_string_buffer(22)

csbi -> console_screen_buffer_info

> Tools/Scripts/webkitpy/common/system/platforminfo.py:98
> +		   res = windll.kernel32.GetConsoleScreenBufferInfo(h, csbi)
> +		   if res:

Nit: Remove the temp variable?

> Tools/Scripts/webkitpy/common/system/platforminfo.py:100
> +		       (_, _, _, _, _, left, _, right, _, _, _) =
struct.unpack("hhhhHhhhhhh", csbi.raw)

Nit: Don't need () on the left side of the assignment.

> Tools/Scripts/webkitpy/layout_tests/views/printing.py:333
> +	   # We don't write into the rightmost column of a terminal since this
may cause lines to wrap on Windows.
> +	   max_line_length = self._meter.number_of_columns() - 1

Can we subtract the extra column only no Windows?  I would be annoyed by the
wasted column on Linux.  Maybe we can do the -1 in number_of_columns to make
the testing easier.