cpython (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation8 Commits4 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

stdout might be a pipe, e.g. less, and then stdin or stderr should be
used to query the terminal for its size.

This patch prefers stdin, since that is most likely connected to a
terminal.

https://bugs.python.org/issue14841

stdout might be a pipe, e.g. less, and then stdin or stderr should be used to query the terminal for its size.

This patch prefers stdin, since that is most likely connected to a terminal.

blueyed added a commit to blueyed/py that referenced this pull request

Apr 5, 2019

This patch prefers stdin, since that is most likely connected to a terminal.

My gut feeling says stderr is more likely to be a terminal than stdin; but it really doesn't matter, because I see all three are being tried in the code.

My gut feeling says stderr is more likely to be a terminal than stdin

Why is that?
I would assume that stdin usually is connected / open for input, while stderr might also be redirected - less likely then stdout, but still.

blueyed added a commit to blueyed/py that referenced this pull request

Aug 17, 2019

blueyed added a commit to blueyed/pytest that referenced this pull request

Nov 1, 2019

Note that os.get_terminal_size might return (0, 0) also, which should be handled (blueyed/pytest#43).
That appears to be an existing issue already, and this PR (or the issue) appears to get no traction, so I will not update it for now - but the linked PR has a fix (I've used the function from this PR there).

Comment on lines +1274 to +1281

for check in [sys.__stdin__, sys.__stderr__, sys.__stdout__]:
try:
size = os_get_terminal_size(check.fileno())
except (AttributeError, ValueError, OSError):
# fd is None, closed, detached, or not a terminal.
continue
else:
break

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for check in [sys.__stdin__, sys.__stderr__, sys.__stdout__]:
try:
size = os_get_terminal_size(check.fileno())
except (AttributeError, ValueError, OSError):
# fd is None, closed, detached, or not a terminal.
continue
else:
break
for stream in (sys.__stdin__, sys.__stderr__, sys.__stdout__):
try:
size = os_get_terminal_size(stream.fileno())
break
except (AttributeError, ValueError, OSError):
# stream is None, closed, detached, or not a terminal.
pass

actual = shutil.get_terminal_size()
self.assertEqual(expected, actual)

# Falls back to stderr.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is decorated with @unittest.skipUnless(os.isatty(sys.__stdout__.fileno()), "not on tty"). What if stderr is not a TTY?


def test_fallback(self):
with support.EnvironmentVarGuard() as env:
del env['LINES']
del env['COLUMNS']

# sys.__stdout__ has no fileno()
with support.swap_attr(sys, '__stdout__', None):
# stdin/stderr/stdout have no fileno().

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# stdin/stderr/stdout have no fileno().
# stdin/stderr/stdout have no fileno() method

the terminal connected to sys.__stdout__ is queried
by invoking os.get_terminal_size.
the terminal connected to sys.__stdin__, sys.__stderr__, or sys.__stdout__
is queried by invoking os.get_terminal_size.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to trust ncurses developers and use the order that they suggested:
https://bugs.python.org/issue14841#msg323836

Please add a comment below on the loop iterating on streams, to add a reference to bpo-14841 to explain why in which order streams are tested.

The following commit authors need to sign the Contributor License Agreement:

git@thequod.de