bpo-14841: shutil.get_terminal_size: use stdin/stderr also #12697
Conversation
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.
This uses an improved version of `shutil.get_terminal_width` [1], and also improves the code for before Python 3.3. 1: https://bugs.python.org/issue14841, python/cpython#12697
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. |
Why is that? |
This uses an improved version of `shutil.get_terminal_width` [1], and also improves the code for before Python 3.3. 1: https://bugs.python.org/issue14841, python/cpython#12697
This uses an improved version of `shutil.get_terminal_width`, mainly to look at stdin, stderr, and stdout. Ref: https://bugs.python.org/issue14841 Ref: python/cpython#12697 Rejected/stalled in pylib: pytest-dev/py#219 Suggested to move into pytest in pytest-dev#5056.
|
Note that |
| 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 |
There was a problem hiding this comment.
| 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. |
There was a problem hiding this comment.
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(). |
There was a problem hiding this comment.
| # 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. |
There was a problem hiding this comment.
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.
stdout might be a pipe, e.g.
less, and then stdin or stderr should beused 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