bpo-41818: Add termios.tcgetwinsize(), termios.tcsetwinsize(). Update docs. by 8vasu · Pull Request #23686 · python/cpython

8vasu · 2020-12-08T01:05:23Z

This follows #23536. Also, see #23546, #23740.

tcgetwinsize() and tcsetwinsize() are expected to appear in IEEE Std 1003.1 ("POSIX.1") issue 8 [ the upcoming version of POSIX ] as declarations in <termios.h>; see https://www.austingroupbugs.net/view.php?id=1151#c3856.

NetBSD already has them (thanks to user "kre"):

Update: musl libc also has these functions.
Update2: The patches that I had submitted to add tcgetwinsize() and tcsetwinsize() to the FreeBSD libc have been merged. I am still working on getting them included in glibc.

When POSIX.1 issue 8 is released, termios.tcgetwinsize(), termios.tcsetwinsize() can be updated to utilize the native versions.

Post #23686, #23546, #23740 goals:

add test_winsize() to "Lib/test/test_pty.py";
major revision of "Lib/pty.py", which has not happened since the beginning of the millennium.

Signed-off-by: Soumendra Ganguly soumendraganguly@gmail.com

https://bugs.python.org/issue41818

Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

8vasu · 2020-12-08T01:43:02Z

@ethanfurman @aeros @gpshead @asvetlov

Update1: These functions perform ioctl()+TIOCGWINSZ/TIOCSWINSZ when the constants are available; they throw NotImplementedError when they are not available. I have implemented os.login_tty(); please see #23740.

Update2: Added ioctl()+TIOCGSIZE to termios.tcgetwinsize().

Update3: Added ioctl()+TIOCSSIZE to termios.tcsetwinsize().

Update4: minor change: unlike struct winsize, the SunOS struct ttysize has members of type int; please refer to https://sourceware.org/git/?p=glibc.git;a=blob;f=bits/ioctl-types.h;h=474a52b4358bc1b4fd211d43ac96f03b168833b7;hb=HEAD#l107

Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

8vasu · 2020-12-14T01:02:04Z

@zoulasc Sir, can you please take a look at the changes that I made to termios.c? termios_tcgetwinsize_impl() and termios_tcsetwinsize_impl() are the functions that I mentioned earlier.

Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

zoulasc

LGTM

8vasu · 2020-12-14T20:39:42Z

@zoulasc Thank you for your time.

…etwinsize(). Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

github-actions · 2021-01-26T00:30:44Z

This PR is stale because it has been open for 30 days with no activity.

8vasu · 2021-01-27T06:50:09Z

Still waiting for core review.

github-actions · 2021-02-28T00:26:51Z

This PR is stale because it has been open for 30 days with no activity.

gpshead

I'd ordinarily also ask for unittest coverage... but this is termios which doesn't appear to have any and might be challenging to automate given its purpose is more for interactive things (?). if you have ideas on that front, feel free to add the beginnings of a Lib/test/test_termios.py - but none of us are going to block this PR on that happening. :)

some comments to address below. primarily: use two item tuples instead of lists.

gpshead · 2021-03-15T00:24:08Z

+ }
+
+ PyObject *v;
+ if (!(v = PyList_New(2))) {


When returning a constant number of values, it is more typical to use a tuple than a list. PyTuple_New and equivalent item setting calls. also update the docstrings in the comment above and on the next function to mention "two item tuple" and use ().

@gpshead Sir, thank you for the review. I should mention that I used PyList_New() instead of PyTuple_New() because I wanted this function to resemble the other low level function termios.tcgetattr(), which also returns a list. Do you confirm this change?
Added: I should have been more specific: termios.tcgetattr() also returns a list of fixed length, and that cannot (?) be changed (backward compatibility?). I was wondering if termios.tcgetwinsize() should also behave similarly.

I ended up making the changes!

bedevere-bot · 2021-03-15T00:32:47Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

8vasu · 2021-08-12T11:42:19Z

@gpshead Unfortunately, I am very busy with my PhD math research currently, and I apologize in advance for not having the time to write unittests for termios :(

Fortunately, I had some time to spare during last December, when I had already written unittests for the pty module, and they were merged (#22962, #23536, #23526). The final goal of this PR (and of #23740 , #23546) is to enhance the pty module, the code for which I already have here https://github.com/8vasu/pypty2, and will create a pull request if/when all of the currently open ones are merged so that all the problems in pty are fixed in a systematic fashion :)

@ambv please take a look at https://bugs.python.org/issue41818 if you have time :)

…t two-item tuples instead of lists.

8vasu · 2021-08-14T06:37:57Z

I have made the requested changes; please review again. Thank you for your patience.

bedevere-bot · 2021-08-14T06:38:00Z

Thanks for making the requested changes!

@gpshead: please review the changes made to this pull request.

…quence, with overflow checking.

gpshead · 2021-08-27T02:57:05Z

 output, :const:`TCIOFF` to suspend input, or :const:`TCION` to restart input.


+.. function:: tcgetwinsize(fd)


one follow up PR as I forgot to fix it up here, these need versionadded tags. I'll take care of it.

@gpshead Thank you for making the improvements!

overlooked in #23686

ambv · 2021-08-27T10:03:48Z

Awesome to see this landed 👏🏻👏🏻👏🏻

Add termios.tcgetwinsize(), termios.tcsetwinsize(). Update docs.
78aaac7
Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

the-knights-who-say-ni added the CLA signed label Dec 8, 2020

bedevere-bot added the awaiting review label Dec 8, 2020

Add news, clinic generated code.
0553063
Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

8vasu force-pushed the tty_winsize branch from a2824f9 to 0553063Compare December 8, 2020 01:15

8vasu mentioned this pull request Dec 8, 2020
bpo-41818: Make additions to the tty module #23546
Closed

Improve termios.tcsetwinsize().
0d2d35b
Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

8vasu mentioned this pull request Dec 11, 2020
bpo-41818: Add os.login_tty() for *nix. #23740
Closed

zoulasc reviewed Dec 14, 2020
View reviewed changes

Comment threadModules/termios.c

8vasu added 2 commits December 13, 2020 22:15

Add TIOCGSIZE support to termios.tcgetwinsize()
628b7dc
Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

Add TIOCSSIZE support to termios.tcsetwinsize()
6ef8a29
Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

zoulasc approved these changes Dec 14, 2020
View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Dec 14, 2020

The SunOS struct ttysize had fields of type "int"; update termios.tcg…
91374ea
…etwinsize(). Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

github-actionsBot added the stale Stale PR or inactive for long period of time. label Jan 26, 2021

github-actionsBot removed the stale Stale PR or inactive for long period of time. label Jan 28, 2021

github-actionsBot added the stale Stale PR or inactive for long period of time. label Feb 28, 2021

gpshead requested changes Mar 15, 2021
View reviewed changes

bedevere-bot added awaiting changes and removed awaiting core review labels Mar 15, 2021

gpshead self-assigned this Mar 15, 2021

gpshead added the type-feature A feature request or enhancement label Mar 15, 2021

termios.tcgetwinsize() and termios.tcsetwinsize() should return/accep…
bb0b2b1
…t two-item tuples instead of lists.

bedevere-bot added awaiting change review and removed awaiting changes labels Aug 14, 2021

bedevere-bot requested a review from gpshead August 14, 2021 06:38

Refactor tcsetwinsize to share common code and accept any two item se…
eb0be09
…quence, with overflow checking.

gpshead reviewed Aug 27, 2021
View reviewed changes

Comment threadModules/termios.c Outdated

parenthesis on my if statements :)
5ab0d4c

gpshead approved these changes Aug 27, 2021
View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Aug 27, 2021

gpshead removed the stale Stale PR or inactive for long period of time. label Aug 27, 2021

gpshead approved these changes Aug 27, 2021
View reviewed changes

gpshead merged commit ae224bb into python:mainAug 27, 2021

bedevere-bot removed the awaiting merge label Aug 27, 2021

gpshead reviewed Aug 27, 2021
View reviewed changes

gpshead added a commit that referenced this pull request Aug 27, 2021
bpo-41818: ++ termios versionadded markers. (GH-27987)
245f1f2
overlooked in #23686

8vasu mentioned this pull request Aug 27, 2021
Update ACKS #27988
Merged

8vasu mentioned this pull request Nov 20, 2021
bpo-41818: Add os.login_tty() for *nix. #29658
Merged

8vasumannequin mentioned this pull request May 5, 2022
Lib/pty.py major revision #85984
Open

mattip mentioned this pull request Nov 11, 2024
termios.tcget/setwinsize pypy/pypy#5108
Closed

		output, :const:`TCIOFF` to suspend input, or :const:`TCION` to restart input.


		.. function:: tcgetwinsize(fd)

Uh oh!

Uh oh!

Conversation

8vasu commented Dec 8, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

8vasu commented Dec 8, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

8vasu commented Dec 14, 2020

Uh oh!

Uh oh!

zoulasc left a comment

Choose a reason for hiding this comment

Uh oh!

8vasu commented Dec 14, 2020

Uh oh!

github-actionsBot commented Jan 26, 2021

Uh oh!

8vasu commented Jan 27, 2021

Uh oh!

github-actionsBot commented Feb 28, 2021

Uh oh!

gpshead left a comment

Choose a reason for hiding this comment

Uh oh!

gpsheadMar 15, 2021

Choose a reason for hiding this comment

Uh oh!

8vasuAug 12, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

8vasuAug 14, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bedevere-bot commented Mar 15, 2021

Uh oh!

8vasu commented Aug 12, 2021

Uh oh!

8vasu commented Aug 14, 2021

Uh oh!

bedevere-bot commented Aug 14, 2021

Uh oh!

Uh oh!

gpsheadAug 27, 2021

Choose a reason for hiding this comment

Uh oh!

8vasuAug 28, 2021

Choose a reason for hiding this comment

Uh oh!

ambv commented Aug 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

8vasu commented Dec 8, 2020•
edited
Loading

8vasu commented Dec 8, 2020•
edited
Loading

8vasuAug 12, 2021•
edited
Loading