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.

Sign up for GitHub

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

Jump to bottom

gh-114328: tty cbreak mode should not alter ICRNL #114335

Merged
merged 3 commits into from
Jan 21, 2024
Merged

gh-114328: tty cbreak mode should not alter ICRNL #114335

merged 3 commits into from
Jan 21, 2024

Conversation

gpshead
Copy link
Member

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

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.

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

@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 bug and security fixes labels Jan 19, 2024
@gpshead gpshead self-assigned this Jan 19, 2024
@bedevere-app bedevere-app bot mentioned this pull request Jan 19, 2024
Closed
@8vasu
Copy link
Contributor

8vasu commented Jan 20, 2024

@gpshead, please see #114328 (comment)

@gpshead gpshead marked this pull request as draft January 20, 2024 20:11
@gpshead gpshead marked this pull request as ready for review January 20, 2024 21:41
@gpshead
Copy link
Member Author

gpshead commented Jan 20, 2024

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

serhiy-storchaka
serhiy-storchaka reviewed Jan 20, 2024
View reviewed changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a 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?

ronaldoussoren
ronaldoussoren approved these changes Jan 21, 2024
View reviewed changes
Copy link
Contributor

@ronaldoussoren ronaldoussoren left a comment

Choose a reason for hiding this comment

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

LGTM

@gpshead
Copy link
Member Author

gpshead commented Jan 21, 2024

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.

@gpshead gpshead merged commit fd49e22 into python:main Jan 21, 2024
32 checks passed
@miss-islington-app
Copy link

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

@gpshead gpshead deleted the lineprinters-its-the-70s branch January 21, 2024 23:25
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 <greg@krypto.org>
@bedevere-app
Copy link

bedevere-app bot commented Jan 21, 2024

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

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and 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 <greg@krypto.org>
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

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