Skip to content
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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

bpo-14841: shutil.get_terminal_size: use stdin/stderr also #12697

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+36 −12
Open

bpo-14841: shutil.get_terminal_size: use stdin/stderr also #12697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cd96f2d
bpo-14841: shutil.get_terminal_size: use stdin/stderr also
blueyed Apr 5, 2019
c614adc
tests
blueyed Apr 5, 2019
364a192
fixup! bpo-14841: shutil.get_terminal_size: use stdin/stderr also
blueyed Apr 5, 2019
5baa778
check for os.get_terminal_size explicitly
blueyed Apr 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions Lib/shutil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1242,8 +1242,8 @@ def get_terminal_size(fallback=(80, 24)):
the value is a positive integer, it is used.

When COLUMNS or LINES is not defined, which is the common case,
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.
Copy link
Member

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.


If the terminal size cannot be successfully queried, either because
the system doesn't support querying, or because we are not
Expand All @@ -1267,11 +1267,20 @@ def get_terminal_size(fallback=(80, 24)):
# only query if necessary
if columns <= 0 or lines <= 0:
try:
size = os.get_terminal_size(sys.__stdout__.fileno())
except (AttributeError, ValueError, OSError):
# stdout is None, closed, detached, or not a terminal, or
# os.get_terminal_size() is unsupported
os_get_terminal_size = os.get_terminal_size
except AttributeError:
size = os.terminal_size(fallback)
else:
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
Comment on lines +1274 to +1281
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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

else:
size = os.terminal_size(fallback)
if columns <= 0:
columns = size.columns
if lines <= 0:
Expand Down
27 changes: 21 additions & 6 deletions Lib/test/test_shutil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2320,24 +2320,39 @@ def test_stty_match(self):
del env['LINES']
del env['COLUMNS']
actual = shutil.get_terminal_size()
self.assertEqual(expected, actual)

self.assertEqual(expected, actual)
# Falls back to stdout.
with support.swap_attr(sys, '__stdin__', None), \
support.swap_attr(sys, '__stderr__', None):
actual = shutil.get_terminal_size()
self.assertEqual(expected, actual)

# Falls back to stderr.
Copy link
Member

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?

with support.swap_attr(sys, '__stdin__', None), \
support.swap_attr(sys, '__stdout__', None):
actual = shutil.get_terminal_size()
self.assertEqual(expected, actual)

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().
Copy link
Member

Choose a reason for hiding this comment

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

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

with support.swap_attr(sys, '__stdin__', None), \
support.swap_attr(sys, '__stderr__', None), \
support.swap_attr(sys, '__stdout__', None):
size = shutil.get_terminal_size(fallback=(10, 20))
self.assertEqual(size.columns, 10)
self.assertEqual(size.lines, 20)

# sys.__stdout__ is not a terminal on Unix
# or fileno() not in (0, 1, 2) on Windows
# stdin/stderr/stdout are not a terminal on Unix,
# or fileno() not in (0, 1, 2) on Windows.
with open(os.devnull, 'w') as f, \
support.swap_attr(sys, '__stdout__', f):
support.swap_attr(sys, '__stdin__', f), \
support.swap_attr(sys, '__stderr__', f), \
support.swap_attr(sys, '__stdout__', f):
size = shutil.get_terminal_size(fallback=(30, 40))
self.assertEqual(size.columns, 30)
self.assertEqual(size.lines, 40)
Expand Down