[PATCH] Detect console width on Windows

[anatoly techtonik]

On Sun, Apr 25, 2010 at 8:09 PM, Patrick Mézard <pmezard at gmail.com> wrote:
>
> I don't really agree with djc on this, I think termwidth() can stay in util.py and be overriden by win32.py. So we don't need an additional fallback.

I do not have any preference for that.

>> +def termwidth():
>> +    screenbuf = win32console.GetStdHandle(win32console.STD_OUTPUT_HANDLE)
>> +    width = 80
>> +    try:
>> +        width = screenbuf.GetConsoleScreenBufferInfo()['Window'].Right
>> +        width += 1
>> +    except pywintypes.error:
>> +        pass
>> +    screenbuf.Detach()
>> +    return width
>
> Two remarks here:
> - We need the COLUMNS logic to override termwidth() from command line (tests use it)

Windows doesn't have any COLUMNS conventions in environment.
Maintaining it for the Linux test seems like a bad idea for me. I am
sure there could be better ways to mock required termwidth() or set it
explicitly.

> - I am tempted to catch exceptions from GetStdHandle() but I have no real use case to backup my paranoia, so this can probably stay for now.

You can get 'The handle is invalid' pywintypes.error, but for what you
would need to do something very nasty to STD_OUTPUT_HANDLE.  I do not
see how to recover from that error in termwidth(), because in this
state any attempt to write to stdout will fail anyway.

> If you agree with all these remarks, just tell me I will update the patch and push it after more tests.

I am comfortable with any decision.


On Sun, Apr 25, 2010 at 9:10 PM, Patrick Mézard <pmezard at gmail.com> wrote:
>
> Here is what they do in bzr-2.0.3:
>
> --------------------
> def get_console_size(defaultx=80, defaulty=25):
>    """Return size of current console.
>
>    This function try to determine actual size of current working
>    console window and return tuple (sizex, sizey) if success,
>    or default size (defaultx, defaulty) otherwise.
>    """
>    if not has_ctypes:
>        # no ctypes is found
>        return (defaultx, defaulty)
>
>    # To avoid problem with redirecting output via pipe
>    # need to use stderr instead of stdout
>    h = ctypes.windll.kernel32.GetStdHandle(WIN32_STDERR_HANDLE)
>    csbi = ctypes.create_string_buffer(22)
>    res = ctypes.windll.kernel32.GetConsoleScreenBufferInfo(h, csbi)
>
>    if res:
>        (bufx, bufy, curx, cury, wattr,
>        left, top, right, bottom, maxx, maxy) = struct.unpack("hhhhHhhhhhh", csbi.raw)
>        sizex = right - left + 1
>        sizey = bottom - top + 1
>        return (sizex, sizey)
>    else:
>        return (defaultx, defaulty)
> --------------------
>
> I tested your current version when piping to less and it actually fails and default to width=80, so I suggest we use WIN32_STDERR_HANDLE instead which is less likely to be redirected.

Checking size of stderr if there is no stdout seems like a non-obvious
hack. It is more logical to remove any limits if autodetection fails
and autodetection is supported on current platform. On Windows
autodetection is supported on all actual platforms. For Linux I do not
know how to differentiate between failure of ioctl call because there
is no such call, and failure because the call itself failed.

> And there is the size computation part, where they take "left" in account while we assume it is zero. I don't how this thing can be changed but I think doing the same is harmless at worse.

In my experiments window size didn't affect print output. Information
is printed to screen buffer, not the window. So if the window
(viewport) is shifted to the right by an amount of chars that is
greater than window width, then you won't see anything at all, because
printed text still starts at 0 position in screen buffer.


On Mon, Apr 26, 2010 at 2:39 PM, Dan Villiom Podlaski Christiansen
<danchr at gmail.com> wrote:
>
> Wouldn't it make more sense to have HGPLAIN affect the terminal width too? The COLUMNS fallback has the annoying effect that changing the terminal width during a long session doesn't affect Mercurial.

What is HGPLAIN? COLUMNS also affect redirected output - another
reason I didn't include their check on Windows.

> Also, the »Progress extension, windows and carriage return« thread suggests that this problem is a general problem on Windows. It seems to me that it would be much better to subtract/not add 1 to the terminal width on Windows. After all, util.termwidth() is much more useful if it consistently means something like the amount of characters that can safely be written to a single terminal line.

I didn't know about this problem. Thanks for CCing me. The thread is
here for future reference:
http://comments.gmane.org/gmane.comp.version-control.mercurial.devel/30655


On Tue, Apr 27, 2010 at 12:12 AM, Patrick Mézard <pmezard at gmail.com> wrote:
>
> I refactored termwidth() on windows then pushed a different version including the annotations in my other responses:
>
> http://hg.intevation.org/mercurial/crew/rev/81631f0cf13b

np. I didn't expect to receive so much valuable feedback. It brings me
much closer to cross-platform pager, which I hope some day will find
its way to standard library.

-- 
anatoly t.