GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-85984: New additions and improvements to the tty library. by 8vasu · Pull Request #101832 · python/cpython

Conversation

@8vasu

@8vasu 8vasu commented Feb 11, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This follows #92365. This is one in a series of PRs aimed at cleaning-up, fixing bugs in, introducing new features in, and updating the code in "Lib/pty.py".

This is a dependency of #101833.

#102413 is another dependency of #101833.

Signed-off-by: Soumendra Ganguly [email protected]

  • Issue: Lib/pty.py major revision #85984

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@8vasu
New additions to the tty library. Functions added: cfmakeraw(), and c…
33f1871
…fmakecbreak(). The

functions setcbreak() and setraw() now return original termios to save an extra tcgetattr() call.

Signed-off-by: Soumendra Ganguly <[email protected]>
@bedevere-bot bedevere-bot added the awaiting review label Feb 11, 2023
@bedevere-bot bedevere-bot mentioned this pull request Feb 11, 2023
Lib/pty.py major revision #85984
Open
@blurb-it
📜🤖 Added by blurb_it.
0bb5baf
@8vasu 8vasu mentioned this pull request Feb 11, 2023
gh-85984: Major revision of the pty library. #101833
Closed
@arhadthedev arhadthedev added the stdlib Standard Library Python modules in the Lib/ directory label Feb 11, 2023
@arhadthedev

arhadthedev commented Feb 11, 2023

Copy link
Copy Markdown
Member

@gpshead as a merger of the previous PR in the series.

8vasu reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@8vasu 8vasu mentioned this pull request Feb 11, 2023
gh-85984: Utilize new "winsize" functions from termios in pty tests. #101831
Merged
8vasu
8vasu commented Feb 11, 2023
View reviewed changes
Comment thread Lib/tty.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@gpshead gpshead self-assigned this Feb 11, 2023
@8vasu

8vasu commented Feb 12, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@gpshead Sorry about the multiple edits.

Goal

To add functionality that lets one easily manipulate a terminal attribute list like the one returned by termios.tcgetattr().
Functionality of the cfget*(), cfset*(), and cfmake*() C functions in the list below must be added. Additionally, our new functions should also be able to set terminal attribute lists to the terminal combination modes listed in the next section.

The following is a list of items of the following format:

*nix tty or pty-related C function : OS/standard : Python function that covers functionality (if any).

A check mark indicates availability in Python.

  • isatty() : POSIX : os.isatty()
  • ttyname() : POSIX : os.ttyname()
  • tcgetattr() : POSIX : termios.tcgetattr()
  • tcsetattr() : POSIX : termios.tcsetattr()
  • tcsendbreak() : POSIX : termios.tcsendbreak()
  • tcdrain() : POSIX : termios.tcdrain()
  • tcflush() : POSIX : termios.tcflush()
  • tcflow() : POSIX : termios.tcflow()
  • tcgetwinsize() : upcoming POSIX : termios.tcgetwinsize()
  • tcsetwinsize() : upcoming POSIX : termios.tcsetwinsize()
  • openpty() : Linux,*BSD,Solaris : os.openpty() and pty.openpty()
  • forkpty() : Linux,*BSD,Solaris : os.forkpty() and pty.fork()
  • login_tty() : Linux,*BSD,Solaris : os.login_tty()
  • cfgetispeed() : POSIX
  • cfgetospeed() : POSIX
  • cfsetispeed() : POSIX
  • cfsetospeed() : POSIX
  • cfsetspeed() : Linux,*BSD
  • cfmakeraw() : Linux,*BSD : None since tty.setraw() applies termios.tcsetattr()
  • cfmakesane() : FreeBSD
  • ptsname : POSIX

Note: Python also includes os.get_terminal_size() which is not *nix-specific.

Edit: Various pty-related functions such as posix_openpt(), getpt(), getpty(), grantpt(), unlockpt() are all covered by os.openpty(). Only having a Python version of ptsname() would be interesting since os.forkpty() does not return the path to the slave end of the pty.

Terminal combination modes

From POSIX manpage for stty:

  • oddp
  • parity, evenp, or oddp
  • raw
  • -raw or cooked
  • nl
  • -nl
  • ek
  • sane

and one from https://en.wikipedia.org/wiki/Terminal_mode:

  • cbreak or rare

Note: Python already includes convenience functions tty.setraw() and tty.setcbreak() but they call termios.tcsetattr() for you. For example, you might want to set your terminal attribute list to raw except that you might want to keep the ECHO flag enabled before applying termios.tcsetattr().

Relevant manpages

  • POSIX: https://pubs.opengroup.org/onlinepubs/9699919799/
  • Linux: https://man7.org/linux/man-pages/man3/termios.3.html
  • FreeBSD: https://man.freebsd.org/cgi/man.cgi?query=cfmakeraw&apropos=0&sektion=3&manpath=FreeBSD+9-current&format=html
  • NetBSD: https://man.netbsd.org/cfmakeraw.3
  • OpenBSD: https://man.openbsd.org/cfmakeraw
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@8vasu
8vasu marked this pull request as draft February 12, 2023 23:09
@8vasu
Merge branch 'main' into tty_new_additions
fb5e28c
@8vasu 8vasu mentioned this pull request Mar 4, 2023
gh-85984: Add POSIX pseudo-terminal functions. #102413
Merged
@gpshead
Add versionadded tags and improve NEWS.
286e30c
gpshead
gpshead approved these changes May 19, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels May 19, 2023
@gpshead
Merge branch 'main' into tty_new_additions
c35b85f
@gpshead
gpshead marked this pull request as ready for review May 19, 2023 17:42
@bedevere-bot bedevere-bot added awaiting review and removed awaiting merge labels May 19, 2023
@gpshead gpshead added the type-feature A feature request or enhancement label May 19, 2023
gpshead added 2 commits May 19, 2023 17:44
@gpshead
versionadded indentation
96c03d5
@gpshead
Merge branch 'tty_new_additions' of https://github.com/8vasu/cpython …
970c419
…into tty_new_additions
@gpshead
gpshead enabled auto-merge (squash) May 19, 2023 17:47
@gpshead
gpshead merged commit 486bc8e into python:main May 19, 2023
@bedevere-bot bedevere-bot removed the awaiting review label May 19, 2023
carljm added a commit to gsallam/cpython_with_perfmap_apii that referenced this pull request May 20, 2023
@carljm
Merge branch 'main' into gsallam/main
ea5965a
* main: (30 commits)
  pythongh-103987: fix several crashes in mmap module (python#103990)
  docs: fix wrong indentation causing rendering error in dis page (python#104661)
  pythongh-94906: Support multiple steps in math.nextafter (python#103881)
  pythongh-104472: Skip `test_subprocess.ProcessTestCase.test_empty_env` if ASAN is enabled (python#104667)
  pythongh-103839: Allow building Tkinter against Tcl 8.7 without external libtommath (pythonGH-103842)
  pythongh-85984: New additions and improvements to the tty library. (python#101832)
  pythongh-104659: Consolidate python examples in enum documentation (python#104665)
  pythongh-92248: Deprecate `type`, `choices`, `metavar` parameters of `argparse.BooleanOptionalAction` (python#103678)
  pythongh-104645: fix error handling in marshal tests (python#104646)
  pythongh-104600: Make type.__type_params__ writable (python#104634)
  pythongh-104602: Add additional test for listcomp with lambda (python#104639)
  pythongh-104640: Disallow walrus in comprehension within type scopes (python#104641)
  pythongh-103921: Rename "type" header in argparse docs (python#104654)
  Improve readability of `typing._ProtocolMeta.__instancecheck__` (python#104649)
  pythongh-96522: Fix deadlock in pty.spawn (python#96639)
  pythonGH-102818: Do not call `PyTraceBack_Here` in sys.settrace trampoline.  (pythonGH-104579)
  pythonGH-103545: Add macOS specific constants for ``os.setpriority`` to ``os`` (python#104606)
  pythongh-104623: Update macOS installer to SQLite 3.42.0 (pythonGH-104624)
  pythongh-104619: never leak comprehension locals to outer locals() (python#104637)
  pythongh-104602: ensure all cellvars are known up front (python#104603)
  ...
@8vasu

8vasu commented May 20, 2023

Copy link
Copy Markdown
Contributor Author

@gpshead Thank you for merging this. However, I have been working on an idea which makes this obsolete. That is why I had converted this PR into a draft (in hindsight, I should have requested you to mark it as DO-NOT-MERGE). Let me explain below.

While working on this PR, I was wondering what the point of the tty module was. On the one hand it looks like it is the place for convenience functions that are not modeled after standard POSIX (or maybe even BSD) functions. On the other hand it imports * from termios, and I have seen people use tty and termios in the same file in an almost interchangeable manner, which seems very "indisciplined" to me.

Upon digging through the source I realized that Steen Lumholt had originally written his 2 functions tty.setraw() and tty.setcbreak() for convenient use in only 2-3 places in Python source (if I recall correctly): a quick grep showed that tty.setraw() is used in the pty module and tty.setcbreak() was used in one other place.

What is more problematic is that the tty module, even after the additions that I wrote for this PR, is highly inflexible. For example, for whatever personal reason, if one wanted to modify some arbitrary attributes, then one would still have to write non-Pythonic and not-so-easily-readable code like this:

import termios
x = termios.tcgetattr(0)                                                                                                                                                                                            

x[3] = x[3] & ~(termios.ECHO|termios.ICANON)
x[2] &= termios.CSIZE
x[2] |= termios.CS8
x[6][termios.VMIN] = 1

termios.tcsetattr(0, termios.TCSANOW, x)

The module that I have been writing is small, free of feature-creep, and uses already standard language provided by stty(1) to do the same as follows:

import stty
x = stty.stty(0)
x.set(echo=False, icanon=False, csize=stty.cs8, min=1)
x.apply(0)

or

import stty
x = stty.stty(0)
x.echo = False
x.icanon = False
x.csize=stty.cs8
x.min=1
x.apply(0)

While I am not a core developer and am not authorized to dictate drastic changes, I am going to request that you consider marking the tty module for deprecation and consider my new module instead. At the very least, maybe just take a look at my source: you can decide whether to keep it or not.

I just defended my PhD dissertation and will be away for 2 weeks for several conferences; when I return after June 10, I will make some final changes to the stty module and upload it here for you to view. Please let me know if this sounds acceptable and if there is any specific procedure to get a new module authorized.

gpshead reacted with thumbs up emoji gpshead reacted with hooray emoji
All reactions
  • 👍 1 reaction
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gpshead

gpshead commented May 20, 2023

Copy link
Copy Markdown
Member

Nice writeup, thanks! I'm not concerned about having merged and added this. They are trivial low level API additions.

We don't have the tty or termios or pty modules on our radar for stdlib deprecation and removal anytime soon (see https://peps.python.org/pep-0594/ for our current list), but it is not unreasonable to think some of that could be considered someday. They're basically un-owned areas of the stdlib with no expert behind them that serve as very low level wrappers over the C APIs (much like what we do in the os module) so they don't provide a "good" interface for anyone to code to. From my POV, you count as the expert at this point. :)

If you do create a nice Pythonic interface stty API, please put it up on PyPI for starters. We could link to that from the stdlib documentation as a suggestion for a nicer interface.

We want to be very conservative on adding major new interfaces to the standard library, and the same with removing old things if they are not a burden. Adding stty would be a valid thing to ask for in the future, but having something up on PyPI to see actual use by others is what we always recommend for starters.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@8vasu

8vasu commented May 20, 2023

Copy link
Copy Markdown
Contributor Author

Completely makes sense! Thank you. I will let you know upon publishing it on PyPI within the next month or so.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra JelleZijlstra mentioned this pull request Sep 28, 2023
gh-85984: Document change in return type of tty functions #110028
Merged
@ronaldoussoren ronaldoussoren mentioned this pull request Jan 19, 2024
read1 in cbreak mode returns 0x0D (instead of 0x0A) when <enter> is entered. #114328
Closed
@8vasu 8vasu mentioned this pull request Feb 4, 2024
gh-85984: Add _POSIX_VDISABLE from unistd.h to termios module. #114985
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@gpshead gpshead

Labels

stdlib Standard Library Python modules in the Lib/ directory type-feature A feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@8vasu @arhadthedev @gpshead @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.