Repository navigation
[PATCH] Make pty.spawn set window size #85713
Description
| BPO | 41541 |
|---|---|
| Nosy | @Yhg1s, @ethanfurman, @8vasu |
| PRs | |
| Files |
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 = None
created_at = <Date 2020-08-13.17:37:00.291>
labels = ['3.8', 'type-bug', 'library', '3.9', '3.10']
title = '[PATCH] Make pty.spawn set window size'
updated_at = <Date 2020-09-15.01:08:45.199>
user = 'https://github.com/8vasu'bugs.python.org fields:
activity = <Date 2020-09-15.01:08:45.199>
actor = 'soumendra'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2020-08-13.17:37:00.291>
creator = 'soumendra'
dependencies = []
files = ['49404', '49455', '49456', '49458']
hgrepos = []
issue_num = 41541
keywords = ['patch']
message_count = 15.0
messages = ['375326', '375366', '375396', '375433', '375438', '375439', '375443', '375445', '375446', '375452', '375461', '375513', '375582', '375631', '375774']
nosy_count = 3.0
nosy_names = ['twouters', 'ethan.furman', 'soumendra']
pr_nums = ['21861']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue41541'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']
Activity
8vasu commentedon Aug 13, 2020
The example in https://docs.python.org/3/library/pty.html that mimics script(1) can be used to reproduce the problem: since window size is not set by pty.spawn, output of "ls" becomes scattered and hard to visually parse if xterm window is not in fullscreen mode.
To fix the above issue, this patch makes pty.spawn set window size ( TIOCSWINSZ ). Also, this patch makes the code of pty.fork() more readable by defining _login_pty(); the latter is reused in spawn().
[-]Make pty.spawn set window size, make code more readable[/-][+]Make pty.spawn set window size[/+][-]Make pty.spawn set window size, make code more readable[/-][+]Make pty.spawn set window size[/+]8vasu commentedon Aug 14, 2020
OS: Linux 4.19.0-9-amd64 Debian 10 GNU/Linux
8vasu commentedon Aug 14, 2020
Note that defining _login_pty() was not a cosmetic change; it is reused in spawn().
8 remaining items
8vasu commentedon Aug 15, 2020
I am new to BPO. Just learned how to make someone nosy.
@twouters, I have attached all resources. This is ready for a review.
Thank you.
8vasu commentedon Aug 15, 2020
Additional note: I am using the i3wm window manager. No desktop environment.
[-]Make pty.spawn set window size [ patch + before, after screenshots ][/-][+][PATCH] Make pty.spawn set window size[/+][-]Make pty.spawn set window size [ patch + before, after screenshots ][/-][+][PATCH] Make pty.spawn set window size[/+]8vasu commentedon Aug 15, 2020
v0.3 removes _login_pty() and defines _login_tty() instead; the latter is based on login_tty(3) from glibc.
8vasu commentedon Aug 15, 2020
v0.4 puts try-except guards around imports so that existing code does not break.
8vasu commentedon Aug 16, 2020
Further proposal: Rename my _login_tty to login_tty and make it available as a part of the pty library. Note that usually login_tty accompanies openpty and forkpty on a system; for example, see
https://www.man7.org/linux/man-pages/man3/login_tty.3.html
https://man.openbsd.org/login_tty
https://netbsd.gw.com/cgi-bin/man-cgi?login_tty++NetBSD-current
However, python's pty only offers openpty and forkpty in the form of pty.openpty and pty.fork respectively. While it is true that forkpty [ pty.fork ] combines openpty, fork, and login_tty, it also closes the slave end of the pty, making it unsuitable for situations where the slave end needs to be kept open; for example, in my patch, the slave end is used to set the window size; or, in case someone wants to do even better and register a SIGWINCH handler for situations in which the window size can change.
8vasu commentedon Aug 18, 2020
Further note: login_tty will also enable us to set slave termios from the parent process in pty.spawn.
Due to the fact that reviewing patches can be overwhelming, v0.5 removes a lot of stuff and instead simply performs window resize by calling ioctl TIOCSWINSZ on the master end of the pty. Still works as expected.
8vasu commentedon Aug 19, 2020
v0.5 had introduced minor mistakes + one hack [ was using master instead of slave to set window size ]. v0.6 removes all such mistakes.
8vasu commentedon Aug 21, 2020
All images, test programs, and old patches have been removed.
window resize test is now being performed using stty.
On linux:
stty -F <terminal-device-file> rows x cols y
On BSDs:
stty -f <terminal-device-file> rows x cols y
to change window size.
kjoyce77 commentedon Nov 17, 2022
Any reason this was dropped? Anyway the patch was useful for me.
8vasu commentedon Mar 7, 2023
@kjoyce77 This was opened as "proof of concept" and was a dirty quick fix (not supposed to be merged). Please comment here instead: #85984. The core developers are very busy (I am not a core developer), and they do the best they can when they get time to review our pull requests. Please help them review my pull requests on that thread if you have time!
encukou commentedon Jan 30, 2024
Superseded by #85984. Let's get that one in :)