jeffkaufman/icdiff

Windows --cols autodetection not working

lorenzogatti opened this issue · 4 comments

Windows 10, Python 3.7.4, mintty (from Mingw-w64)

The --cols parameter is 0 or null, which causes the cols variable to be 0 and the self._wrapcolumn field to be -2.
This nonsensical value causes an infinite loop in _split_line() since variable line2 (i.e. the part of the input that is goes into the next line and is split recursively as needed) is set to the whole input.
Reproducible with any two different, nonempty input files.

Why not offer a sane default value for --cols instead of requiring the user to set the option?

I suggest computing the column count from the sum of the maximum line length of the two input files, so that the layout has the minimum width that doesn't cause text to wrap.

Wide lines in a narrow terminal window would look bad, but it is far better than crashing and in that case the user can rerun icdiff with the --cols option.

The --cols autodetection is: https://github.com/jeffkaufman/icdiff/blob/master/icdiff#L551

It does try to handle windows, but not having a windows machine I can't test it. Would you be able to run the following and let me know what it prints?

import os
import struct
from ctypes import windll, create_string_buffer

print("os.name: %s" % os.name)

fh = windll.kernel32.GetStdHandle(-12)  # stderr is -12
csbi = create_string_buffer(22)
windll.kernel32.GetConsoleScreenBufferInfo(fh, csbi)
res = struct.unpack("hhhhHhhhhhh", csbi.raw)
print("res[7]: %s" % res[7])
print("res[5]: %s" % res[5])

separately, the code is kind of broken here with how it responds to errors; I'll fix that as well

Ok, cleaned up in cad37ad and now if the windows autodetection code has an error we'll fall back to 80 columns. But I'm still curious why the windows autodetection isn't working on your system.

According to mintty/mintty#482, GetConsoleScreenBufferInfo fails under mintty. It returns 0 and all CONSOLE_SCREEN_BUFFER_INFO fields are 0.
It returns 1 and correct values in cmd.exe and Powershell.

Solution: avoid using mintty.

Slightly improved test program:

import os
import struct
from ctypes import windll, create_string_buffer

print("os.name: %s" % os.name)

fh = windll.kernel32.GetStdHandle(-12) # stderr is -12
csbi = create_string_buffer(22)
status=windll.kernel32.GetConsoleScreenBufferInfo(fh, csbi)
print("GetConsoleScreenBufferInfo return value: %i"% status)
print(struct.unpack("hhhhHhhhhhh", csbi.raw))