Issue 13041: argparse: terminal width is not detected properly (original) (raw)

Created on 2011-09-24 22:07 by zbysz, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue13041.patch zbysz,2012-02-22 15:36 use shutil.get_terminal_size() review
Pull Requests
URL Status Linked Edit
PR 8459 merged berker.peksag,2018-07-25 09:11
Messages (12)
msg144507 - (view) Author: Zbyszek Jędrzejewski-Szmek (zbysz) * Date: 2011-09-24 22:07
COLUMNS is a shell variable (updated whenever the window size changes), that usually isn't exported to programs. Therefore checking for COLUMNS in sys.environ will not work in the majority of cases. The proper check is to use the TIOCGWINSZ ioctl on stdout. Why COLUMNS is not exported? Because it can change during the lifetime of a program. Therefore it is better to use the dynamic ioctl.
msg144521 - (view) Author: Zbyszek Jędrzejewski-Szmek (zbysz) * Date: 2011-09-25 10:01
I see that adding a separate module was proposed in issue #8408, which was rejected/closed. I don't have the rights to reopen, so I'll continue here. #8408 was proposing a new module, which seems a bit overkill, since the implementation for unix and windows is about 20 lines. I'm attaching a second version of the patch which works on windows (tested with python3.2.2 on XP). Thanks to techtonik for pointing to a windows imlementation.
msg144523 - (view) Author: Denilson Figueiredo de Sá (denilsonsa) Date: 2011-09-25 14:18
> #8408 was proposing a new module, which seems a bit overkill If a module seems overkill, then maybe add this useful function to os module. Don't leave it private to argparse module. Maybe something along these lines: >>> import os >>> print(os.get_terminal_size()) (80, 25) Why do I believe a module could be better? Because I'd also like some way to detect when the terminal size has changed (without probing it all the time).
msg149531 - (view) Author: Steven Bethard (bethard) * (Python committer) Date: 2011-12-15 11:08
I'd feel more comfortable with the argparse fix if it were simply calling "os.get_terminal_size()". I recommend that you: * Create a new issue called, say "add os.get_terminal_size()" proposing just the single method. * Add that issue to the Dependencies of this issue. Once that is fixed, then the argparse fix should be simple.
msg149587 - (view) Author: Denilson Figueiredo de Sá (denilsonsa) Date: 2011-12-16 01:05
Issue #13609 created, but I don't have permission to edit the dependencies.
msg150728 - (view) Author: Zbyszek Jędrzejewski-Szmek (zbysz) * Date: 2012-01-06 13:04
New version to use after #13609 is implemented: patch2.diff
msg153961 - (view) Author: Zbyszek Jędrzejewski-Szmek (zbysz) * Date: 2012-02-22 15:36
OK, I guess that this could now be closed, since 13609 has been commited. (It is currently reopened, but the proposed tweaks wouldn't influence the usage in argparse, even if accepted). I'm attaching a patch which updates the tests to the new $COLUMNS logic. Previously, unsetting COLUMNS would fix the width on 80, now setting COLUMNS=80 is the proper way to do this.
msg223222 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-07-16 15:29
@Paul the attached patch is extremely simple and follows the work on #13609. Is it okay with you if the patch was committed?
msg223298 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2014-07-16 23:30
The latest patch, using _shutil.get_terminal_size(), looks fine. It lets environ['COLUMNS'] have priority over the end user's terminal width, as demonstrated by the change to test_argparse. test_argparse doesn't test changing the actual terminal size, but I imagine that would be a pain to implement.
msg223809 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2014-07-24 05:24
For now the user could add this to his module: import os, shutil os.environ['COLUMNS'] = str(shutil.get_terminal_size().columns)
msg315274 - (view) Author: Julian Mehnle (jmehnle) Date: 2018-04-13 23:58
What's holding up the merging of this patch?
msg322365 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2018-07-25 15:23
New changeset 74102c9a5f2327c4fc47feefa072854a53551d1f by Berker Peksag in branch 'master': bpo-13041: Use shutil.get_terminal_size() in argparse.HelpFormatter (GH-8459) https://github.com/python/cpython/commit/74102c9a5f2327c4fc47feefa072854a53551d1f
History
Date User Action Args
2022-04-11 14:57:21 admin set github: 57250
2018-07-25 15:24:20 berker.peksag set status: open -> closedstage: patch review -> resolvedresolution: fixedversions: + Python 3.8, - Python 3.5
2018-07-25 15:23:47 berker.peksag set messages: +
2018-07-25 09:11:28 berker.peksag set pull_requests: + <pull%5Frequest7983>
2018-04-14 00:09:54 BreamoreBoy set nosy: - BreamoreBoy
2018-04-13 23:58:03 jmehnle set nosy: + jmehnlemessages: +
2014-07-24 05:24:13 paul.j3 set messages: +
2014-07-16 23:30:18 paul.j3 set messages: +
2014-07-16 20:15:50 berker.peksag set nosy: + berker.peksagtype: behavior -> enhancementversions: + Python 3.5, - Python 3.3
2014-07-16 15:29:44 BreamoreBoy set nosy: + paul.j3, BreamoreBoymessages: +
2012-02-22 15:36:48 zbysz set files: - patch2.diff
2012-02-22 15:36:42 zbysz set files: - patch1.1.diff
2012-02-22 15:36:33 zbysz set files: - patch1.diff
2012-02-22 15:36:05 zbysz set files: + issue13041.patchmessages: +
2012-01-06 13:04:38 zbysz set files: + patch2.diffmessages: +
2011-12-16 12:12:27 giampaolo.rodola set dependencies: + Add "os.get_terminal_size()" functionversions: - Python 2.7, Python 3.2
2011-12-16 01:05:43 denilsonsa set messages: +
2011-12-15 11:08:45 bethard set messages: + versions: + Python 2.7, Python 3.2
2011-10-27 21:55:35 flox set nosy: + floxstage: patch reviewversions: - Python 3.4
2011-09-25 14🔞42 denilsonsa set messages: +
2011-09-25 10:02:00 zbysz set files: + patch1.1.diffmessages: +
2011-09-25 02:40:21 denilsonsa set nosy: + denilsonsa
2011-09-24 22:13:22 ezio.melotti set nosy: + bethard
2011-09-24 22:12:15 ezio.melotti link issue13042 superseder
2011-09-24 22:07:18 zbysz create