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 #13041 can be trivially fixed. There are some suggestions about how to implement this feature in issue #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)

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

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

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.

[1]: http://bugs.python.org/file23241/patch1.1.diff

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.

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.

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

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

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.

> 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.

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.

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.

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")

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).

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

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

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.

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

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 issue12442.
- 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 / issue12442)


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

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)

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 issue12442.
> - 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 / issue12442)

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

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.

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).

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).

> 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.

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.

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).

> 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);

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.

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.

One more comment on $COLUMNS overriding the actual terminal size:

> 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.

To fix issue #9553 "test_argparse.py: 80 failures if COLUMNS env var set to a value other than 80", sys.env['COLUMNS']=80 is set during tests.
If issue #13041 is fixed and a real terminal size is used, then
unless $COLUMNS have higher preference, the tests would break again.

Some remarks on the Windows implementation in termsize.diff.4:

- On Windows, the C runtime always sets fileno(stdout) to 1, so hardcoded values are OK.
But on Unix, I'm quite sure that embedded interpreters (mod_python?) sometimes close the standard descriptor, so fd=1 can refer to something entirely different.
Does it makes sense to initialize fd=fileno(stdout) (this is C code) instead?

- When GetStdHandle() returns INVALID_HANDLE_VALUE, PyErr_SetFromWindowsErr(0) should be used.
And it's not necessary to use GetLastError(), 0 is enough.

- GetStdHandle will return NULL in a non-console application (try with pythonw.exe or IDLE), I think a specific error message should be raised in this case.

> Some remarks on the Windows implementation in termsize.diff.4:

> - On Windows, the C runtime always sets fileno(stdout) to 1, so
> hardcoded values are OK.
> But on Unix, I'm quite sure that embedded interpreters (mod_python?) 
> sometimes close the standard descriptor, so fd=1 can refer to 
> something entirely different.
> Does it makes sense to initialize fd=fileno(stdout) (this is C code) 
> instead?
OK, I agree that fd=fileno(stdout) is better. Now it matches the docstring which says "defaults to stdout" better.

> - When GetStdHandle() returns INVALID_HANDLE_VALUE, 
> PyErr_SetFromWindowsErr(0) should be used.
OK.
> And it's not necessary to use GetLastError(), 0 is enough.
OK, good no know.

> - GetStdHandle will return NULL in a non-console application (try 
> with pythonw.exe or IDLE), I think a specific error message should be 
> raised in this case.
OK. I tried and I see that the GetStdHandle documentation says so, but
I couldn't get it to return NULL. Nevertheless, I added a check:
+        if (handle == NULL)
+            return PyErr_Format(PyExc_OSError, "stdout not connected");

Thanks for the review!

Updated: termsize.diff.5

> - fd argument is retained, because we might want to test terminals
>  opened with openpty.

You mean a terminal different than the one used for stdin, stdout and stderr?

> - 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.

get_terminal_size() looks like reliable than raw_get_terminal_size()
because environment variables are not updated when the terminal is
resized.

>> - fd argument is retained, because we might want to test terminals
>>  opened with openpty.
>
> You mean a terminal different than the one used for stdin, stdout and 
> stderr?

For example, let's see what is the size of my two xterms:

>>> os.get_terminal_size_raw()
os.terminal_size(columns=125, rows=39)
>>> fd = os.open('/proc/22736/fd/1', os.O_RDONLY)
>>> fd
6
>>> os.get_terminal_size_raw(6)
os.terminal_size(columns=95, rows=33)

I'm not saying that this serves a clear purpose, but it is possible
and it is good not the restrain the API. I'm sure somebody could find
a use for it.

>> - 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.
>
> get_terminal_size() looks like [less?] reliable than raw_get_terminal_size()
> because environment variables are not updated when the terminal is
> resized.
Please look at my comment above: http://bugs.python.org/issue13609#msg149620
$COLUMNS is only used for overriding -- it normally _isn't_ set.

Does this need need more discussion, code review, testing, or just more time?

> Does this need need more discussion, code review, testing,
> or just more time?

As I already wrote, I would prefer a very simple os.get_terminal_size() function: don't read environment varaiables, use a simple tuple instead of a new type, and raise an error if the size cannot be read (so no need of default values). The os module is written as a thin wrapper between Python and the OS. A more high level function (read environment variables, handle the error, use a namedtuple) can be written in your application, or maybe in another module.

This is just my opinion, other core developers may prefer your version :-)

> > Does this need need more discussion, code review, testing,
> > or just more time?
> 
> As I already wrote, I would prefer a very simple
> os.get_terminal_size() function: don't read environment varaiables,
> use a simple tuple instead of a new type, and raise an error if the
> size cannot be read (so no need of default values). The os module is
> written as a thin wrapper between Python and the OS. A more high level
> function (read environment variables, handle the error, use a
> namedtuple) can be written in your application, or maybe in another
> module.

I think we have reached the point where we won't be in total agreement
over the API, so let's choose whatever is submitted as a patch.

I only have two remaining issues with the patch:
- the tests needn't be in a separate file, they can go in test_os
- there should be a test for get_terminal_size_raw as well (and of
course it should be skipped if the function doesn't exist)

> read environment varaiables [...] and raise an error if the size cannot be
> read (so no need of default values). The os module is written as a thin
> wrapper between Python and the OS. A more high level function (read 
> environment variables, handle the error, use a namedtuple) can be written in 
> your application, or maybe in another module.

+1. I also find weird that a function, especially one living in the os module, has such a high level of abstraction (basically this is why I was originally proposing shutil module for this to go in).

Given the different opinions about the API, I think it's best to expose the lowest level functionality as-is, and let the user decide what to do (read env vars first, suppress the exception, use a fallback, etc.). 


> I think we have reached the point where we won't be in total 
> agreement over the API, so let's choose whatever is submitted 
> as a patch.

I'd be more careful. Once this gets in it will be too late for a change.

> +1. I also find weird that a function, especially one living in the os
> module, has such a high level of abstraction (basically this is why I
> was originally proposing shutil module for this to go in).
> 
> Given the different opinions about the API, I think it's best to
> expose the lowest level functionality as-is, and let the user decide
> what to do (read env vars first, suppress the exception, use a
> fallback, etc.). 

Fair enough, but other people expressed sympathy for the two-function
approach :) I'm personally indifferent, although I find
"get_terminal_size_raw" a bit ugly and liked "query_terminal_size"
better.

(and looking up ROWS and COLUMNS make sense, since they are de facto
standards)

On Sat, Jan 21, 2012 at 17:40, Giampaolo Rodola' <report@bugs.python.org> wrote:
>
> Given the different opinions about the API, I think it's best to expose the lowest
> level functionality as-is, and let the user decide what to do (read env vars first,
> suppress the exception, use a fallback, etc.).

As a Python user (and not a committer), I disagree.

As an user, I don't care too much where the function should be placed
(although I believe os or sys are sensible choices). What I do care is
that I want a extremely simple function that will "just work". Don't
make me add code for handling all the extra cases, such code should be
inside the function.

All this discussion about the API made me remember this presentation:
http://python-for-humans.heroku.com/

Also, I see no downside of using a Named Tuple. Issue 4285 actually
added a named tuple to the sys.version_info.

> As a Python user (and not a committer), I disagree.
>
> As an user, I don't care too much where the function should be placed
> (although I believe os or sys are sensible choices). What I do care is
> that I want a extremely simple function that will "just work". Don't
> make me add code for handling all the extra cases, such code should be
> inside the function.

For what it's worth, as a Python committer, I agree with you.
Python is a very high level language, and I think the standard library
should be as natural and offer the same productivity gain as the core
language does.
Exposing to the user a mere wrapper around a syscall/library just
doesn't make sense to me. Sure, it should be made available for those
who want/need to do low-level system programming (and I'm one of
those), but the vast majority of users want something higher level
than the POSIX/Windows library.

Here's is an updated version: termsize.diff.6

Following Antoine Pitrou's comment get_terminal_size_raw is renamed back to query_terminal_size.

Tests are moved to test_os.py, and there's a new test for query_terminal_size -- the output from 'stty size' is parsed. If something fails (e.g. stty is not available), the test is skipped.

I noticed that bash uses $LINES, not $ROWS. I have renamed rows to lines everywhere, to follow existing convention.

The big remaining question seems to be one function vs. two functions.
I'm still convinced that the high-level two function is better. This issue originated in the argparse module, which can use proper terminal size to efficiently format messages. Of course argparse must use a function with a fallback -- if it cannot query, some default must be used. So argparse would implement the high-level function anyway. It might just as well be in os, available for others to be used. Anyway, the second function is 18 lines of python code (excluding docstring and whitespace) -- a bit too much to copy and paste.

> I noticed that bash uses $LINES, not $ROWS. I have renamed rows to lines everywhere, to follow existing convention.

The stty program has "rows" and "columns" commands to set the terminal
size. The tput programs has "cols" and "lines" commands to get the
terminal size. The curses library uses (y, x), e.g. it has a
getmaxyx() function. I don't know the most common term, lines or rows.

Extract of http://stackoverflow.com/questions/1780483/lines-and-columns-environmental-variables-lost-in-a-script
"The variables $LINES and $COLUMNS are shell variables, not
environmental variables, and thus are not exported to the child
process (...)"

If I understood correctly, LINES and COLUMNS environment variables are
only set by the user. In a bash script, these variables are wrapper to
ioctl().

The tput program reads TERM environment variable to decide if LINES
and COLUMNS are used or not:
"-Ttype indicates  the type of terminal.  Normally this option is
unnecessary, because the default is taken from the environment
variable TERM.  If -T is specified, then the shell variables LINES and
COLUMNS will be ignored,and the operating system will not be queried
for the actual screen size."

Extract of http://pubs.opengroup.org/onlinepubs/7908799/xbd/envvar.html :
------
COLUMNS
    A decimal integer > 0 used to indicate the user's preferred width
in column positions for the terminal screen or window. (See column
position .) If this variable is unset or null, the implementation
determines the number of columns, appropriate for the terminal or
window, in an unspecified manner. When COLUMNS is set, any
terminal-width information implied by TERM will be overridden. Users
and portable applications should not set COLUMNS unless they wish to
override the system selection and produce output unrelated to the
terminal characteristics. The default value for the number of column
positions is unspecified because historical implementations use
different methods to determine values corresponding to the size of the
screen in which the utility is run. This size is typically known to
the implementation through the value of TERM or by more elaborate
methods such as extensions to the stty utility, or knowledge of how
the user is dynamically resizing windows on a bit-mapped display
terminal. Users should not need to set this variable in the
environment unless there is a specific reason to override the
implementation's default behaviour, such as to display data in an area
arbitrarily smaller than the terminal or window.

LINES
    A decimal integer > 0 used to indicate the user's preferred number
of lines on a page or the vertical screen or window size in lines. A
line in this case is a vertical measure large enough to hold the
tallest character in the character set being displayed. If this
variable is unset or null, the implementation determines the number of
lines, appropriate for the terminal or window (size, terminal baud
rate, and so forth), in an unspecified manner. When LINES is set, any
terminal-height information implied by TERM will be overridden. Users
and portable applications should not set LINES unless they wish to
override the system selection and produce output unrelated to the
terminal characteristics. The default value for the number of lines is
unspecified because historical implementations use different methods
to determine values corresponding to the size of the screen in which
the utility is run. This size is typically known to the implementation
through the value of TERM or by more elaborate methods such as
extensions to the stty utility, or knowledge of how the user is
dynamically resizing windows on a bit-mapped display terminal. Users
should not need to set this variable in the environment unless there
is a specific reason to override the implementation's default
behaviour, such as to display data in an area arbitrarily smaller than
the terminal or window.
------

> The big remaining question seems to be one function vs. two functions.

Not exactly. I suggested to keep only the simple function in the os
module, and add maybe a function with a higher level API (using
environment variables) in another module (e.g. in shutil).

Updated version following comments by Victor Stinner: termsize.diff.7

- os.get_terminal_size() is moved to shutil.get_terminal_size()
- some small doc updates

Victor STINNER wrote:
>> I noticed that bash uses $LINES, not $ROWS. I have renamed rows to
>> lines everywhere, to follow existing convention.
> The stty program has "rows" and "columns" commands to set the terminal
Yes, also struct winsize uses 'rows' and 'cols'. But since we use the variable $LINES, it seems better to use 'lines'.

Thanks for the link to SUSv2! I've added a link in the docs, since it adds a nice justification to the COLUMNS/query/fallback behaviour.

Now get_terminal_size() lives in shutil. I don't think it matters much, but I see the point of keeping os clean.

I also looked at some programs, to see who behaves how.
dpkg -> COLUMNS first
aptitude -> ignores COLUMNS
git -> COLUMNS first
systemctl, loginctl, systemd-cgls (systemd cmdline interface) -> COLUMNS first
less, w ->  ioctl, COLUMNS as fallback

(Of course this is very unscientific, because I just picked some programs at random which I remembered to care about the terminal size).
I guess that the case of 'less' is special, because less is very
much dependent on the terminal and does quite a lot of manipulations on it. Otherwise, taking COLUMNS as highest priority seems well established, even if not universal.

Terminal stuff is irrelevant to `shutil`, which is the module for 'High-level file operations' and deserve a separate module named 'console'.

Why? Because terminal size is only relevant when you want to page output. The next step would be to detect if terminal supports color to see if ANSI sequences will be collapsed. Then you'll want to get user input without pressing '<enter>' key.

It is a whole new module.

"except Exception" clauses in the tests are too broad. Otherwise, looks fine.

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?

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 #444582 and 
#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

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

> 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.

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)?

"The env. var. is for overriding."

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

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.

> +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.

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

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

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.

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.

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?

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.

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.

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

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

> 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.

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().

> 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.

> 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.

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.

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

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?

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.

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