Add "os.get_terminal_size()" function

[denilsonsa]

BPO	13609
Nosy	@loewis, @amauryfa, @pitrou, @vstinner, @giampaolo, @merwok
Files	termsize.diff termsize.diff.1 termsize.diff.2: third version of the patch, after review termsize.diff.3: fourth version of the patch, after more reviews termsize.diff.4 termsize.diff.5: sixth version of the patch, after review termsize.diff.6 termsize.diff.7: eighth version of the patch termsize.diff.8 get_terminal_size_pipe.patch get_terminal_size_pipe-2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2012-05-16.15:43:07.521>
created_at = <Date 2011-12-16.01:01:38.409>
labels = ['type-feature', 'library']
title = 'Add "os.get_terminal_size()" function'
updated_at = <Date 2012-05-16.15:43:07.520>
user = 'https://bugs.python.org/denilsonsa'

bugs.python.org fields:

activity = <Date 2012-05-16.15:43:07.520>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2012-05-16.15:43:07.521>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2011-12-16.01:01:38.409>
creator = 'denilsonsa'
dependencies = []
files = ['23981', '23983', '24117', '24127', '24150', '24157', '24369', '24373', '24379', '24503', '24504']
hgrepos = []
issue_num = 13609
keywords = ['patch']
message_count = 71.0
messages = ['149586', '149595', '149597', '149613', '149616', '149617', '149620', '149627', '149629', '149630', '149631', '149653', '150287', '150388', '150419', '150454', '150456', '150462', '150474', '150475', '150481', '150689', '150693', '150695', '150701', '150703', '150715', '150722', '150731', '150732', '150759', '150775', '150776', '151391', '151729', '151740', '151742', '151749', '151755', '151772', '152330', '152334', '152360', '152388', '152389', '152390', '152394', '152395', '152396', '152397', '152398', '152807', '152808', '152913', '152914', '152915', '153184', '153185', '153231', '153232', '153233', '153235', '153236', '153237', '153238', '153380', '153382', '153390', '153494', '153495', '160877']
nosy_count = 13.0
nosy_names = ['loewis', 'amaury.forgeotdarc', 'pitrou', 'vstinner', 'techtonik', 'giampaolo.rodola', 'eric.araujo', 'Arfrever', 'zbysz', 'denilsonsa', 'neologix', 'rosslagerwall', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue13609'
versions = ['Python 3.3']

[denilsonsa]

Please add a function called "os.get_terminal_size()" that should return a tuple "(width, height)".

The built-in "argparse" module would directly benefit from this function, as it would wrap the help text correctly. I'm pretty sure many users would also benefit from it.

Once this function gets implemented, issue bpo-13041 can be trivially fixed. There are some suggestions about how to implement this feature in issue bpo-8408.

[extra feature:] After this function gets implemented, it might be a good idea to implement some kind of API to detect size changes without requiring probing. (or, at least, the documentation should give some directions about how to achieve it)

[pitrou]

Well, do you want to propose a patch yourself?
See http://docs.python.org/devguide/ for how to contribute a patch.

[zbysz]

The patch is already almost there (in bpo-13041). I'll post an updated version here in a moment.

[denilsonsa]

Zbyszek, I just looked at 1 and I disagree that the environment variable should have higher precedence. In fact, I believe it should have lower precedence, and should be used as a fallback.

Reason? Imagine a program is launched, and thus it has COLUMNS set to some value. While the program is running, the user resizes the terminal. I believe (though I have not tested) that the envvar will keep the old value, and this function would return wrong dimensions.

In my opinion, the system-specific code (termios/windll) should be tried first, and then the envvars (as fallback). If all else fails, maybe it would be better to return (None, None) and let the caller of this function handle that.

[giampaolo]

http://bugs.python.org/file23241/patch1.1.diff
This looks like something which would fit better into shutil module rather than os.
Also, the Windows implementation should not rely on ctypes.

[giampaolo]

Plus, you should provide also heigth, not only width, possibly as a namedtuple:

>>> import shutil
>>> shutil.get_terminal_size()
size(width=80, heigth=170)
>>>

[zbysz]

One reply to three comments:

[To myself]:

I'll post an updated version here in a moment.
Hm, it's not so simple. The implementation is simple,
but the configure defines are harder than I thought.
But I'm getting there :)

Zbyszek, I just looked at [1] and I disagree that the environment variable should have higher precedence. In fact, I believe it should have lower precedence, and should be used as a fallback.

I disagree. $COLUMNS will usually _not_ be set:

$ echo $COLUMNS
119

$ ./python -c 'import os; print("COLUMNS" in os.environ)'
False

As I wrote before, the shell only sets COLUMNS for use in the shell, without exporting. Giving a precedence to the environment variable (if it is set by the user) allows to do something like this:
$ COLUMNS=80 python program.py
... do whatever is to be dome for 80 columns ...
and the normal case (with dynamic checking) will also work correctly.

Anyway, I think that two functions should be provided: a "raw" one, which does the ioctl, and the "normal" one, which queries $COLUMNS and the the "raw" function. If different behaviour is wanted, than just
the "raw" one can be called.

> Also, the Windows implementation should not rely on ctypes.
Of course. For windows I have:
  #include <conio.h>
  ...
  GetConsoleScreenBufferInfo(handle, &csbi)
  ...

This looks like something which would fit better into shutil module rather than os.
This is problematic. The docstring for shutil says:
Utility functions for copying and archiving files and directory trees.
So it doesn't seem to fit at all.

OTOH, there's termios and tty, but they are UNIX only. Module curses is also UNIX only, and slightly different topic, because get_terminal_width should be independent of curses.

Plus, you should provide also heigth, not only width, possibly as a namedtuple.
Agreed.

[giampaolo]

The docstring for shutil says: Utility functions for copying and
archiving files and directory trees. So it doesn't seem to fit at all.

Well... shutil should stand for "shell utilities" and it already contains stuff which is not strictly related to file/directory direct operations (see shutil.disk_usage and upcoming shutil.which).

But maybe you're right... I don't know...
My point is that a function which attempts different fallbacks (ask OS -> ask os.environ -> return (None, None) / raise exception) sounds more like an utility function rather than something belonging to os module, where you tipically see 1-to-1 interfaces for the underlying system functions.

[zbysz]

This is proof-of-concept implementation.

This adds two modules: termsize (python) and _termsize (C). The first one contains the get_terminal_size user-facing function and namedtuple definition. The second on contains the function query_terminal_size which does the real job.

Two simple tests are added. I'm not sure if it is feasible to test with different terminal sizes: it certainly _could_ be done, e.g. by launching an xterm with a specified geometry, but this would be much
more complicated than this implementation, so probably not worth it.

This was only tested on 64-bit linux, seems to work.

I'm not sure how to the configure tests should be done: right now the presence of <sys/ioctl.h> is checked, and it is used. If not available,
<conio.h> is checked, and used. Otherwise NotImplementedError is thrown.

Would be nice to test on a mac and other systems, but I don't have one available at the moment unfortunately.

I think that the python part (termsize.py) should be either merged with os.py (or whatever home we find for this function), or rewritten in C in _termsizemodule.c and only imported into os. But since it hasn't yet been decided where this should go, keeping it separate is easier for now.

[amauryfa]

Don't forget the Windows platform... here is an implementation: https://bitbucket.org/hpk42/py/src/980c8d526463/py/_io/terminalwriter.py#cl-284
But it would be better written in C, of course.

[pitrou]

Ok, a couple of general comments:

• there is no point having a separate module for a single function; I think the os module (and posixmodule.c for the C side) is a reasonable place where to put this
• C code should be indented with 4 spaces increments and no tabs (see PEP-7)
• constants in C code should be uppercase
• C code should be C89-compliant and therefore we don't use named struct initializers (such as ".m_size = 0")

[zbysz]

Here's updated version: termsize.diff.1

Ok, a couple of general comments:

• there is no point having a separate module for a single function; I > think the os module (and posixmodule.c for the C side) is a
  reasonable place where to put this
  Done. (But posixmodule.c is so enourmous... I feel bad making it even longer.)

• C code should be indented with 4 spaces increments and no tabs (see > PEP-7)
• constants in C code should be uppercase
• C code should be C89-compliant and therefore we don't use named
  struct initializers (such as ".m_size = 0")
  All done, I hope.

This version was tested on linux/amd64 and win32 (XP).

[zbysz]

Seems to work also on kfreebsd/debian (with eglibc).

[pitrou]

(review posted on http://bugs.python.org/review/13609/show )

[zbysz]

Thanks for the review!

New version is attached. The code is actually slightly shorter, but
there are more docs.

Doc/library/os.rst | 52 +++++++++++++++++++
Doc/whatsnew/3.3.rst | 5 +
Lib/os.py | 43 +++++++++++++++
Lib/test/test_termsize.py | 31 +++++++++++
Misc/NEWS | 3 +
Modules/posixmodule.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++
configure | 2
configure.in | 2
pyconfig.h.in | 3 +
9 files changed, 264 insertions(+), 2 deletions(-)

The patch also lacks some documentation update in Doc/library/os.rst.
Added as a subsection under "File Descriptor Operations" section.
I also added an entry to Misc/NEWS and Doc/whatsnew/3.3.rst.

Lib/os.py:833: def get_terminal_size(columns=80, rows=25):
The arguments seem ignored in the function body, so I think they should simply
be removed.
The implementation was borked and didn't actually do what the
docstring said. It should be fixed now.

I want to retain the default values, because get_terminal_size()
should "do the right thing" in case stdout is not a terminal,
without the user needing to wrap it in a try..except block
or some other test. Putting the default values as parameters
makes it clear what will be returned in case query fails, and
makes it easy to override those defaults.

To make it clearer that those are the fallback values, I renamed the
parameter to 'fallback=(c, r)', which should be understandable even
without reading the docstring. Having it as a single parameter might
also make it easier to add something like 'minimum=(c, r)' in the
future.

I wonder if you shouldn't try to explicitly pass sys.stdout.fileno() here, or
perhaps sys.__stdout__.fileno() since the former could be overriden.
OK, changed to sys.__stdout__.fileno().

I think that sys.__stdout__ is better, because when stdout is overridden,
it is usually with something that is not a terminal. Changing the output
terminal is probably pretty rare.

Actually, perhaps get_terminal_size() should have an optional file descriptor
argument.
Querying anything other than stdout should be pretty rare. If
necessary, query_terminal_size() can be called explicitly. Also,
the variables $COLUMNS and $ROWS are usually meant to refer to stdout,
so get_terminal_size() is about stdout and ROWS and COLUMNS, and the
other function allows full control.

Lib/test/test_termsize.py:1: import unittest
This file seems to lack an if __name__ == '__main__' like other test files
OK.

Besides, why not simply put the tests in test_os.py?
The tests were nicely self-contained. I wanted to be able to do:
./python Lib/test/test_termsize.py
and
./python Lib/test/test_termsize.py | cat
but I guess it can be easily moved to test_os.py if necessary.

Lib/test/test_termsize.py:7: def set_environ(**variables):
There's already EnvironmentVarGuard in test.support.
OK.

Lib/test/test_termsize.py:7: def set_environ(**variables):
There's already EnvironmentVarGuard in test.support.
Lib/test/test_termsize.py:25: self.assertTrue(0 < size.columns)
Better to use assertGreater here, failure messages will be more informative.
Lib/test/test_termsize.py:33: self.assertTrue(size.columns == 777)
And better to use assertEqual here.
OK.

Typo here ("imlmented").
OK.

Modules/posixmodule.c:10571: return PyErr_Format(PyExc_ValueError)
I don't think there's any point in changing the error here. Just let
a normal OSError be raised.
OK, s/ValueError/OSError/ here and the the windows case below too.

Modules/posixmodule.c:10573: return PyErr_SetFromErrno(PyExc_IOError);
For the record, in 3.3, PyExc_IOError is an alias of PyExc_OSError.
OK, s/IOError/OSError/ for clarity.

Modules/posixmodule.c:10599: return PyErr_Format(PyExc_IOError, "error %i",
(int) GetLastError());
Just use PyErr_SetFromWindowsErr(GetLastError()).
OK.

Modules/posixmodule.c:11086: {"query_terminal_size", query_terminal_size,
METH_VARARGS, termsize__doc__},
I don't think there's any point in making this helper function public, so I'd
rename it _query_terminal_size or something.
The idea is that query_terminal_size() can be used to do the
low-level query ignoring $COLUMNS and $ROWS and returing the real
error if something goes wrong. If is documented, so I think that
it can be "public".

I've also improved the docstrings slightly.

[pitrou]

Thanks for the updated patch.
I think I would like other people's opinions about the dual-function approach.

[giampaolo]

What I would do:

• build the namedtuple in Python rather than in C
• I don't particularly like CamelCased names for nametuples (I would use "size" instead of "TerminalSize")
• on UNIX, the ioctl() stuff can be done in python rather than in C
• use C only on Windows to deal with GetStdHandle/GetConsoleScreenBufferInfo; function name should be accessed as nt._get_terminal_size similarly to what we did for shutil.disk_usage in bpo-12442.
• do not raise NotImplementedError; if the required underlying functions are not available os.get_terminal_size should not exists in the first place (same as we did for shutil.disk_usage / bpo-12442)

Also, I'm not sure I like fallback=(80, 25) as default, followed by:

    try:
        size = query_terminal_size(sys.__stdout__.fileno())
    except OSError:
        size = TerminalSize(fallback)

That means we're never going to get an exception.
Instead I would:

• provide fallback as None
• let OSError propate unless fallback is not None

[rosslagerwall]

Nice patch :-)

I think the two function approach works well.

Since you have already checked that termsize is not NULL, Py_DECREF can be used instead of Py_CLEAR.

Would it not be better to use sys.__stdout__ instead of 1 in the documentation and to use STDOUT_FILENO instead of 1 in the code?

A brief google suggested that Solaris requires sys/termios.h for TIOCGWINSZ. Perhaps also only define TERMSIZE_USE_IOCTL if TIOCGWINSZ is defined.
Like so:
#if defined(HAVE_SYS_IOCTL_H)
#include <sys/ioctl.h>
#if defined(TIOCGWINSZ)
#define TERMSIZE_USE_IOCTL
#else
#define TERMSIZE_USE_NOTIMPLEMENTED
#endif
#elif defined(HAVE_CONIO_H)
#include <windows.h>
#include <conio.h>
#define TERMSIZE_USE_CONIO
#else
#define TERMSIZE_USE_NOTIMPLEMENTED
#endif

(didn't check the windows parts)

[zbysz]

Thanks for the reviews!

• build the namedtuple in Python rather than in C
  I started this way, but if two functions are used, it is nicer to have them both
  return the same type. If it was defined in the Python part, there would be
  loop in the sense that os would import the function from _posixmodule, and
  _posixmodule would import the namedtuple from os. The code for the tuple is fairly simple,
  mostly docs, and saving a few lines just doesn't seem to be worth the complication of doing
  it other way around.

• I don't particularly like CamelCased names for nametuples (I would use "size" instead of "TerminalSize")
  I was following the style in _posixmodule: there is WaitidResultType and SchedParamType.
  "size" seems to generic, maybe something like "terminal_size" would be better (SchedParamType
  is exported as sched_param).

Will change to "terminal_size".

• on UNIX, the ioctl() stuff can be done in python rather than in C
  It can, but it requires extracting the bytes by hand, doing it in C is cleaner.
  The C implementation is only 5 lines (in addition to the code necessary for windows)
  and again it seems simpler this way. Also the configuration is kept in one
  place in the #ifdefs at the top, not in two different files.

• use C only on Windows to deal with GetStdHandle/GetConsoleScreenBufferInfo; function name should be
  accessed as nt._get_terminal_size similarly to what we did for shutil.disk_usage in bpo-12442.
• do not raise NotImplementedError; if the required underlying functions are not available
  os.get_terminal_size should not exists in the first place (same as we did for shutil.disk_usage / bpo-12442)

I think that there is a difference in "importance" -- shutil.disk_usage cannot be
faked, or ignored, and if it is unavailable, then some work-around must be implemented.
OTOH, the terminal size is an embellishment, and if a wrong terminal size is used, the output
might wrap around or be a bit narrow (the common case), but like in argparse.FancyFormatter,
just continuing is probably reasonable. So to save the users of the function the trouble
to wrap it in some try..except, let's cater for the common case.

This same argument goes for defining the function only if an implementation is available:
argparse would need something like:
try:
os.get_terminal_size
except NameError:
def get_terminal_size():
return (80, 25)
which just doesn't seem to _gain_ anything.

Also, I'm not sure I like fallback=(80, 25) as default, followed by:
[...]
That means we're never going to get an exception.
Instead I would:

• provide fallback as None
• let OSError propate unless fallback is not None
  Again, if the user forgets to add the fallback, and only tests in a terminal, she
  would get an unnecessary surprise when running the thing in a pipe. So the fallback
  would have to be almost always provided... and it will almost always be (80, 25).
  So let's just provide it upfront, and let the user call the low-level function if
  they want full control.

-----------------------------------------------------------------------------------

I think the two function approach works well.
:)

Since you have already checked that termsize is not NULL, Py_DECREF can be used instead of Py_CLEAR.
OK.

Would it not be better to use sys.__stdout__ instead of 1 in the documentation and to use STDOUT_FILENO
instead of 1 in the code?
OK.

A brief google suggested that Solaris requires sys/termios.h for TIOCGWINSZ. Perhaps also only define
TERMSIZE_USE_IOCTL if TIOCGWINSZ is defined.
Hm, I don't have solaris available, so I won't be able to check easily, but
maybe sys/termios.h should be imported if TIOCGWINSZ is not available. Would
be nice to provide the implementation if possible.

Like so:
Something like:

#if defined(HAVE_SYS_IOCTL_H)
#include <sys/ioctl.h>
#if defined(TIOCGWINSZ)
#define TERMSIZE_USE_IOCTL
#else

#if defined(HAVE_SYS_TERMIOS_H)
#include <sys/termios.h>
#if defined(TIOCGWINSZ)
#define TERMSIZE_USE_IOCTL
#else
#define TERMSIZE_USE_NOTIMPLEMENTED

#endif
#elif defined(HAVE_CONIO_H)
#include <windows.h>
#include <conio.h>
#define TERMSIZE_USE_CONIO
#else
#define TERMSIZE_USE_NOTIMPLEMENTED
#endif

[rosslagerwall]

I'll try & investigate Solaris a bit...

Also, what should be the default terminal size?

Gnome-terminal and xterm seem to default to 80x24, not 80x25.

[zbysz]

Updated patch termsize.diff.3

• s/TerminalSize/terminal_size/ in Python code
• change the fallback to (80, 24) (seems to be the commonest default)
• s/Py_CLEAR/Py_DECREF/
• use STDOUT_FILENO
• add more hrefs in docs
• include <termios.h> is available (untested fix for solaris compatibility)
• rename TerminalSize as terminal_size in Python code

I tested this iteration only on linux and windows, but it is not substantially changed, so should still work on *bsd. (I previously
tested on debian/kfreebsd and dragonflybsd and it seemed functional).

[loewis]

I haven't read much of this issue, but I strongly dislike the use of named tuples. Either we want people to use named fields, then we should use a regular class (possibly with slots), or we want to define the result as two values, then there should be a plain tuple result.

named tuples should only be used as a compatibility mechanism, when the first design used tuples, and it was later found that additional values need to be returned which would change the number of values (or the original design was considered bad because it returned too many positional values to properly keep track).

[zbysz]

I haven't read much of this issue, but I strongly dislike the use of
named tuples.
I don't really have a very strong opinion, but (cols, rows) does seem a lot like a tuple -- it really is just a pair of values without other function or state. Still I would much prefer to say
get_terminal_size().columns
than
get_terminal_size()[0]
So a bare tuple seems like the worst choice.

[pitrou]

The point of named tuples here is that you can use both
get_terminal_size().columns
or
columns, rows = get_terminal_size()
depending on the situation.
Also, the better repr() makes debugging easier.

[vstinner]

Some comments about termsize.diff.3.

I don't see why there are two functions, one should be enough: get_terminal_size() should be dropped, and query_terminal_size() renamed to get_terminal_size(). As said before, I don't think that reading ROWS and COLUMNS environment variables is useful. If a program chose to rely on these variables, it can reimplement its own "try env var or fallback on get_terminal_size()" function.

get_terminal_size() should not have a fallback value: it should raise an error, and the caller is responsible to decide what to do in this case (e.g. catch the exception and use its own default value). Most functions in the posix module work like this, and it avoids the difficult choice of the right default value. (fallback=None is an hack to avoid an exception, it's not the pythonic.)

I don't think that it's possible that stdin, stdout and/or stderr have its own terminal. I suppose that the 3 streams are always attached to the same terminal. So I don't see why the function would take an argument. Tell me if I am wrong.

Instead of using sys.__stdout__.fileno(), you can directly use 1 because Python always create sys.__stdout__ from the file descriptor 1.

I think that a tuple (columns, rows) would be just fine. A namedtuple helps when you have a variable number of fields, or more than 3 fields, but here you just have 2 fields, it's not too much difficult to remember which one contains the columns.

I would prefer an optional function, instead of implementing a function raising a NotImplementedError. All other posix functions are defined like this.

ioctl() is already exposed in the fcntl module, I don't see a specific test for <sys/ioctl.h> header. It looks like the module is always compiled on Unix, I don't see how fcntl and ioctl are tested in setup.py.

I don't think that you need <conio.h> to get GetConsoleScreenBufferInfo(), <windows.h> should be enough. So just check for "#ifdef MS_WINDOWS".

Your function is helpful, and it is surprising that nobody proposed to implement it in Python. Some libraries did already implement their own function (like the "py" library).

[pitrou]

I don't think that it's possible that stdin, stdout and/or stderr have
its own terminal. I suppose that the 3 streams are always attached to
the same terminal. So I don't see why the function would take an
argument. Tell me if I am wrong.

I think it can be useful in case the program creates its own
session/terminal using openpty?

Instead of using sys.__stdout__.fileno(), you can directly use 1
because Python always create sys.__stdout__ from the file descriptor
1.

From pythonrun.c:

/* Set sys.stdin */
fd = fileno(stdin);
[...]
/* Set sys.stdout */
fd = fileno(stdout);
[...]
/* Set sys.stderr, replaces the preliminary stderr */
fd = fileno(stderr);

[loewis]

Zitat von Antoine Pitrou <report@bugs.python.org>:

Antoine Pitrou <pitrou@free.fr> added the comment:

The point of named tuples here is that you can use both
get_terminal_size().columns
or
columns, rows = get_terminal_size()
depending on the situation.

And my point is that we should make the choice which
of these is more obvious, and drop the other.

Also, the better repr() makes debugging easier.

A class could still have a nice repr.

[zbysz]

Following comments by Martin and Victor, here is next version: termsize.diff.4

Changes:

• just check for defined(MS_WINDOWS) and rely on <windows.h>.
• rename query_terminal_size to get_terminal_size_raw
  This way it should be clearer that the second one is low-level,
  and should be less exposed. I don't want to call it
  _get_terminal_size() because it is not just an implementation
  detail and would sometimes be called directly.
• NotImplementedError is gone. get_terminal_size_raw() is not
  defined if not possible.

Non-changes:

• sys.__stdout__.fileno() is not changed to 1, because as Antoine
  pointed out, it is set at runtime.
• STDOUT_FILENO: not defined on windows, so just use 1 and add a comment
• fd argument is retained, because we might want to test terminals
  opened with openpty.
• two functions: still there. I think that get_terminal_size() should
  provide an easy-to-use, even trivial-to-use, way to get a sensible
  value without writing a wrapper. In the _majority_ of cases the
  wrapper would be something like get_terminal_size() currently.
• named tuple: like Antoine said, it gives nice syntax.
• testing for <sys/ioctl.h>: in termios and other modules, setup.py
  first tests if we are on unix. But there might be unices without
  TIOCGWINSZ, and non-unix systems where TIOCGWINSZ _is_ defined,
  so it seems a functional test is simpler and more robust.

Tested on linux and windows XP.

[vstinner]

termsize.diff.7:

• shutil.get_terminal_size(): I would prefer columns and lines
  variable names instead of "ccc" and "rrr"
• "Right now the values are not cached, but this might change." why
  would it be cached? this sentence can just be removed
• I would prefer os.get_terminal_size() instead of
  os.query_terminal_size(), I didn't know other function using the verb
  "query" in Python
• To keep os simple, os.query_terminal_size() can return a simple,
  whereas shutil.get_terminal_size() returns a namedtuple
• test_os.py: use @unittest.skipUnless() on TermsizeTests to check if
  os.query_terminal_size() is available
• test.py, test_does_not_crash() catchs OSError, you may only ignore
  some error codes, not any. For example, skip the test if stdout is not
  a tty

+ try:
+ size = os.query_terminal_size(sys.__stdout__.fileno())
+ except (NameError, OSError):
+ size = os.terminal_size(fallback)

AttributeError should be catched here, not NameError. Or better, check
if os has a query_terminal_size() function.

+.. function:: get_terminal_size(fallback=(columns, lines))

Hum, you may document the default value: (80, 24).

shutil.get_terminal_size() doesn't allow to choose the file
descriptor. Would it be useful to allow to get the size of sys.stderr
or another TTY?

[zbysz]

Well, right now it's just one function. Functionality which you propose
could of course be useful, but let's leave if for another day, another
issue.

See also http://bugs.python.org/issue13609#msg149627 and bpo-444582 and
bpo-12442 -- shutil is growing in different directions, and terminal size
can be one of them.

> Antoine Pitrou added the comment:
> "except Exception" clauses in the tests are too broad.
Fixed.
> Otherwise, looks fine.

> STINNER Victor added the comment:
> - shutil.get_terminal_size(): I would prefer columns and lines
> variable names instead of "ccc" and "rrr"
Changed (this was a leftover from the version when there was no named
tuple).

> - I would prefer os.get_terminal_size() instead of
> os.query_terminal_size(), I didn't know other function using the verb
> "query" in Python
Changed. I used os.g_t_s and shutil.g_t_s in docs so it should be clear
which is which.

> - To keep os simple, os.query_terminal_size() can return a simple,
> whereas shutil.get_terminal_size() returns a namedtuple
We would have two very similar functions returning something different.
Also, the amount of code saved will be negligible, because the named
tuple is mostly docs. I prefer to keep it in os to keep both functions
similar.

> - test_os.py: use @unittest.skipUnless() on TermsizeTests to check if
> os.query_terminal_size() is available

• To keep os simple, os.query_terminal_size() can return a simple,
  whereas shutil.get_terminal_size() returns a namedtuple
  AttributeError should be catched here, not NameError. Or better, check
  if os has a query_terminal_size() function.
  Fixed. I changed the tests a bit, e.g. to take into account that stty
  queries stdin, not stdout.

> Hum, you may document the default value: (80, 24).
Done.

shutil.get_terminal_size() is tied to COLUMNS and ROWS and thus makes
most sense for stdout. But I guess that an optional parameter could be
added:
shutil.get_terminal_size(fd=None, fallback=(80, 24))
where fd could be either an integer, or an object with .fileno().
I don't know.

> - "Right now the values are not cached, but this might change." why
> would it be cached? this sentence can just be removed
Done. In theory it could be cached with watching for SIGWINCH, but I'll
just remove the comment for now.

Thanks for the review and comments!

Patch version nine attached: termsize.diff.8

[techtonik]

What happens with COLUMNS env variables if terminal is resized after the Python script is executed. Will get_terminal_size() return new value?

[zbysz]

the Python script is executed. Will get_terminal_size()
> return new value?
Please see previous discussion and the docs (and the SUSv2 specs).
The env. var. is for overriding.

[techtonik]

On Tue, Jan 31, 2012 at 7:48 PM, Zbyszek Szmek <report@bugs.python.org>wrote:

Zbyszek Szmek <zbyszek@in.waw.pl> added the comment:

> the Python script is executed. Will get_terminal_size()
> return new value?
Please see previous discussion and the docs (and the SUSv2 specs).
The env. var. is for overriding.

Could you just answer the question or provide a relevant link
(unfortunately I don't have time to read SUSv2 specs)?

[zbysz]

"The env. var. is for overriding."

http://bugs.python.org/issue13609#msg149620
http://bugs.python.org/issue13609#msg152334

[techtonik]

All right, I've found some time to grep conversation related to COLUMNS/ROWS environment/shell variable.

+1 for low level system wrapper to get current stdout console size
-1 on COLUMN/ROWS "business logic"

My user story 001:
I need exact values of my console and don't want them to be overridden by environment/shell variables.
-- or --
When troubleshooting problems with console width on user side and don't want this feature to implicitly depend on some variables in user environment.

[vstinner]

+1 for low level system wrapper to get current stdout console size

So use os.get_terminal_size()

-1 on COLUMN/ROWS "business logic"

So don't use shutil.get_terminal_size(), but it looks like their is a
use case for this feature.

[python-dev]

New changeset c92f8de398ed by Antoine Pitrou in branch 'default':
Issue bpo-13609: Add two functions to query the terminal size:
http://hg.python.org/cpython/rev/c92f8de398ed

[pitrou]

The patch is finally committed. Thank you Zbyszek for having been quite patient.
(according to Murphy's laws, this will surely break some buildbots)

[zbysz]

Thanks for merging!

I'll try to keep an eye on the buildbot results, but please add me to
any bugs in case I miss something.

[Arfrever]

test_stty_match() should be skipped also when os.isatty(sys.__stdin__.fileno()) is False.
This test fails when test suite is run by Portage (Gentoo package manager), which somehow sets null input.

[zbysz]

Hi,

looking at the tests, the test should be skipped with 'stty invocation
failed'. Does something different happen? Can you post the output from
the tests?

[Arfrever]

Actually this test fails due to another reason. I'm still investigating it.

In the meantime, a different bug was found:

# stty size | cat
46 157
# python3.3 -c 'import os; print(os.get_terminal_size())' | cat
Traceback (most recent call last):
  File "<string>", line 1, in <module>
OSError: [Errno 22] Invalid argument

Especially redirection to less is often used. stty size works, so it should be possible to fix Python.

[vstinner]

Using strace, I see that stty calls ioctl(TIOCGWINSZ) on stdin (fd=0)
if it failed on stdout (fd=1), whereas Python only tries stdout.

Attached patch implements a similar idea.

[pitrou]

Your patch is wrong. It always discards ioctl(stdout), even if it was successful.

[vstinner]

New try (I fixed my email address and the patch).

[zbysz]

Using strace, I see that stty calls ioctl(TIOCGWINSZ) on stdin (fd=0)
if it failed on stdout (fd=1), whereas Python only tries stdout.
It was done this way by design. Maybe checking stdin can be also useful,
but it is a rather big change in semantics. I think it should be a separate bug.

It is pretty common for programs to behave differently when run through pipe, even if stdin is on a tty. stty is rather the exception than the rule. E.g. almost all programs disable color when piped explicitly through less. 'dpkg | cat' ignores terminal width. So does git and ls.
stty is special, because the only purpose of that program is to query terminal size, but it cannot be taken as a model for the behaviour of a general purpose program.

[Arfrever]

The purpose of os.get_terminal_size() is the same as stty size, so stty size could be a model for behavior of os.get_terminal_size().

[vstinner]

E.g. almost all programs disable color when piped explicitly through less.

Using my patch, you can use os.get_terminal_size(sys.stdout.fileno()) if you like to get an error if sys.stdout is a pipe. My patch only changes the behaviour if no argument is specified (hum, the "special" behaviour should be documented). Or you can also check explicitly sys.stdout.isatty().

It is just more convinient to fallback to stdin if stdout is not a TTY.

[zbysz]

New try (I fixed my email address and the patch).

The purpose of os.get_terminal_size() is the same as stty size, so
stty size could be a model for behavior of os.get_terminal_size().
I guess that this fallback will make some things easier. As long it is documented, the user should be able to get whatever behaviour they want.

I have four small questions:

• -1 is used as the argument meaning "try stdout and stdin". But -1 is
  also used the error return value for open(2) and other system
  functions. I know that Python checks the return value to throw an
  exception, and -1 shouldn't "leak" and be passed by mistake to
  os.get_terminal_size(). Can someone confirm that this is
  the case?
• Why stderr is not checked too? It would make os.get_terminal_size()
  return the real terminal value in a the middle of a pipe:
  cat | python -c 'import os; os.get_terminal_size()' | cat.
• some doc change is missing
• in the new version the line with ioctl(fd, TIOCGWINSZ, &w) repeats
  three times. It think it could be reduced to two without loss
  of readability.

[denilsonsa]

On Tue, Feb 14, 2012 at 22:17, Zbyszek Szmek <report@bugs.python.org> wrote:

I have four small questions:

• -1 is used as the argument meaning "try stdout and stdin".
• Why stderr is not checked too?

I propose the following solution: accept either an integer or a
sequence of integers as the fd parameter. In the second case, try each
FD until one of them returns the terminal size.

[zbysz]

Wouldn't this be quite unwieldy to use:
os.get_terminal_size(sys.__stdout__.fileno(),
sys.__stdin__().fileno(),
sys.__stderr__.fileno())
?

[pitrou]

I don't think there's much point in the proposed complications.
If you're willing to know the terminal size, you're probably interested in displaying something in it (using stdout), so why would you care about stderr or stdin?

[zbysz]

Stdout can be connected to a pipe, e.g to less, which in turn might
be connected to a terminal. The program can then display output properly
scaled for the terminal, assuming that because stdin is
connnected to a terminal, output will eventually reach the same
terminal. Sometimes this is true, sometimes not.

[pitrou]

I am closing as fixed. If you want to propose further enhancements, please open a new issue.