support alternative file descriptors in get_terminal_width

[RonnyPfannschmidt]

base for pytest-dev/pytest#2640

when we move the terminal around we break our neck

[Victorious3]

With the change to use shutil.get_terminal_size() this is still a problem. It doesn't return the correct terminal width if stdout is redirected (However, os.get_terminal_size() does...) I get this on Windows now (Using conftest.py on a test session with pytest):

>>> shutil.get_terminal_size()
os.terminal_size(columns=80, lines=24)
>>> os.get_terminal_size()
os.terminal_size(columns=117, lines=71)
>>> os.get_terminal_size(sys.__stdout__.fileno()) # This is what shutil is trying to call
ValueError: bad file descriptor
>>> sys.__stdout__.isatty() # However...
True

From what I can tell shutil is supposed to "break" when there's no terminal (piping, or whatever) This is the only discussion I found about it: https://groups.google.com/forum/#!topic/comp.lang.python/v9VszdDzpdE

I don't know of a good reason to not use os.get_terminal_size but someone might disagree with me. It's 2018 and terminals still suck, apparently.

[RonnyPfannschmidt]

terminals are technically unable to un-suck ^^ its simply unfixable because its technology from the far past ^^

[Victorious3]

Anything I could do about this, then? It's now half broken on Windows, it's using the correct terminal width on pytest -s and is still stuck to 80 characters wide when capturing stdout.

[RonnyPfannschmidt]

off hand im not sure where the fix is best placed, also this is python 3.3+ only as far as i understood

but bascially the terminalwriter could always use its own fd in some way

[Victorious3]

@nicoddemus What do you think about this?

[nicoddemus]

Hi @Victorious3,

I don't know of a good reason to not use os.get_terminal_size but someone might disagree with me.

os.get_terminal_size's own docs mention that shutil.get_terminal_size should be used instead:

shutil.get_terminal_size() is the high-level function which should normally be used, os.get_terminal_size is the low-level implementation.

I think the reason is that shutil.get_terminal_size() implements support to manually override the terminal size by setting COLUMNS and LINES:

For each of the two dimensions, the environment variable, COLUMNS and LINES respectively, is checked. If the variable is defined and the value is a positive integer, it is used.

The implementation also matches it: https://github.com/python/cpython/blob/fcd5e84a515e19409840c570730f0728e9fcfc83/Lib/shutil.py#L1203

So IIUC using os.get_terminal_size would not fix anything, I'm afraid.

[Victorious3]

Well, shutil is always using sys.__stdout__.fileno(), which doesn't work if the stdout is redirected. How about checking COLUMNS and LINES beforehand, instead of using it as a fallback, and then querying os.get_terminal_size() (with the original stdout?)

[RonnyPfannschmidt]

technically this is "another" bug in pytest/capture - since we force-dup the fd away, we ought to fixup sys.stdout with a correct handle we might need to get tricky there

[nicoddemus]

How about checking COLUMNS and LINES beforehand

It already does that: https://github.com/python/cpython/blob/fcd5e84a515e19409840c570730f0728e9fcfc83/Lib/shutil.py#L1224

technically this is "another" bug in pytest/capture - since we force-dup the fd away,

I'm not very familiar with how duplicating file handles work, but duplicating it doesn't keep the original valid?

[RonnyPfannschmidt]

@nicoddemus we do a targeted dup2 into the fds for stdout/err - thats pretty much destruction there

[Victorious3]

Until this gets fixed...

def pytest_configure(config):
    # HACK
    shutil.get_terminal_size = os.get_terminal_size

works like a charm ^^

[RonnyPfannschmidt]

@nicoddemus basically we need to monkeypatch sys.__stdout__ fun times ahead