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-114328: tty cbreak mode should not alter ICRNL by gpshead · Pull Request #114335 · python/cpython

Conversation

@gpshead

@gpshead gpshead commented Jan 19, 2024 •
edited by github-actions Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

The terminal CR -> NL mapping setting should be inherited in cbreak mode as OSes do not specify altering it as part of their stty cbreak mode definition.

  • Issue: read1 in cbreak mode returns 0x0D (instead of 0x0A) when <enter> is entered. #114328

📚 Documentation preview 📚: https://cpython-previews--114335.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gpshead
pythongh-114328: tty cbreak mode should not alter ICRNL.
0302768 
CR -> NL mapping should be inherited in cbreak mode as OSes do not
specify altering it as part of their `stty` cbreak mode definition.
@gpshead gpshead added type-bug An unexpected behavior, bug, or error needs backport to 3.12 only security fixes labels Jan 19, 2024
@gpshead gpshead self-assigned this Jan 19, 2024
@bedevere-app bedevere-app Bot added the awaiting core review label Jan 19, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Jan 19, 2024
read1 in cbreak mode returns 0x0D (instead of 0x0A) when <enter> is entered. #114328
Closed
@gpshead
NEWS entry
3c28745
@gpshead
gpshead requested a review from serhiy-storchaka January 19, 2024 23:50
@8vasu

8vasu commented Jan 20, 2024

Copy link
Copy Markdown
Contributor

@gpshead, please see #114328 (comment)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gpshead
gpshead marked this pull request as draft January 20, 2024 20:11
@bedevere-app bedevere-app Bot removed the awaiting core review label Jan 20, 2024
@gpshead
Clarify the specific behavior in the docs.
d1b0823
@gpshead
gpshead marked this pull request as ready for review January 20, 2024 21:41
@bedevere-app bedevere-app Bot added the awaiting core review label Jan 20, 2024
@gpshead

gpshead commented Jan 20, 2024

Copy link
Copy Markdown
Member Author

Updated further after conversation on the issue, see the discussion there. This should be ready.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gpshead
gpshead requested a review from ronaldoussoren January 20, 2024 21:43
serhiy-storchaka
serhiy-storchaka reviewed Jan 20, 2024
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is considered a bugfix, why versionchanged is needed?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
ronaldoussoren
ronaldoussoren approved these changes Jan 21, 2024
View reviewed changes

@ronaldoussoren ronaldoussoren left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Jan 21, 2024
@gpshead

gpshead commented Jan 21, 2024

Copy link
Copy Markdown
Member Author

If it is considered a bugfix, why versionchanged is needed?

It's unclear if it is really needed (we normally don't), but it seems nice for our users to call the known issue out for a while. I'm okay with if these get deleted at a some later date.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gpshead
gpshead merged commit fd49e22 into python:main Jan 21, 2024
@miss-islington-app

miss-islington-app Bot commented Jan 21, 2024

Copy link
Copy Markdown

Thanks @gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gpshead
gpshead deleted the lineprinters-its-the-70s branch January 21, 2024 23:25
@bedevere-app bedevere-app Bot removed the awaiting merge label Jan 21, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 21, 2024
@gpshead @miss-islington
pythongh-114328: tty cbreak mode should not alter ICRNL (pythonGH-114335
e8b93e0 
)

The terminal CR -> NL mapping setting should be inherited in cbreak mode as OSes do not specify altering it as part of their stty cbreak mode definition.
(cherry picked from commit fd49e22)

Co-authored-by: Gregory P. Smith <[email protected]>
@bedevere-app

bedevere-app Bot commented Jan 21, 2024

Copy link
Copy Markdown

GH-114410 is a backport of this pull request to the 3.12 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Jan 21, 2024
gpshead added a commit that referenced this pull request Jan 21, 2024
@miss-islington @gpshead
[3.12] gh-114328: tty cbreak mode should not alter ICRNL (GH-114335) (#…
33c1907 
…114410)

The terminal CR -> NL mapping setting should be inherited in cbreak mode as OSes do not specify altering it as part of their stty cbreak mode definition.
(cherry picked from commit fd49e22)

Co-authored-by: Gregory P. Smith <[email protected]>
kulikjak pushed a commit to kulikjak/cpython that referenced this pull request Jan 22, 2024
@gpshead @kulikjak
pythongh-114328: tty cbreak mode should not alter ICRNL (python#114335)
baa4afe 
The terminal CR -> NL mapping setting should be inherited in cbreak mode as OSes do not specify altering it as part of their stty cbreak mode definition.
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@gpshead @aisk
pythongh-114328: tty cbreak mode should not alter ICRNL (python#114335)
f421909 
The terminal CR -> NL mapping setting should be inherited in cbreak mode as OSes do not specify altering it as part of their stty cbreak mode definition.
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@gpshead @Glyphack
pythongh-114328: tty cbreak mode should not alter ICRNL (python#114335)
0dc3c29 
The terminal CR -> NL mapping setting should be inherited in cbreak mode as OSes do not specify altering it as part of their stty cbreak mode definition.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
@ronaldoussoren ronaldoussoren ronaldoussoren approved these changes

Uh oh!

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

Assignees

@gpshead gpshead

Labels

type-bug An unexpected behavior, bug, or error

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

@gpshead @8vasu @ronaldoussoren @serhiy-storchaka
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.