Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-85984: New additions and improvements to the tty library. #101832

Merged
merged 7 commits into from
May 19, 2023

Conversation

8vasu
Copy link
Contributor

@8vasu 8vasu commented Feb 11, 2023

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 soumendraganguly@gmail.com

…fmakecbreak(). The

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

Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>
@arhadthedev arhadthedev added the stdlib Python modules in the Lib dir label Feb 11, 2023
@arhadthedev
Copy link
Member

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

Lib/tty.py Show resolved Hide resolved
@gpshead gpshead self-assigned this Feb 11, 2023
@8vasu
Copy link
Contributor Author

8vasu commented Feb 12, 2023

@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

@8vasu 8vasu marked this pull request as draft February 12, 2023 23:09
@gpshead gpshead marked this pull request as ready for review May 19, 2023 17:42
@gpshead gpshead added the type-feature A feature request or enhancement label May 19, 2023
@gpshead gpshead enabled auto-merge (squash) May 19, 2023 17:47
@gpshead gpshead merged commit 486bc8e into python:main May 19, 2023
carljm added a commit to gsallam/cpython_with_perfmap_apii that referenced this pull request May 20, 2023
* 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
Copy link
Contributor Author

8vasu commented May 20, 2023

@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
Copy link
Member

gpshead commented May 20, 2023

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.

@8vasu
Copy link
Contributor Author

8vasu commented May 20, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants