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-118911: Trailing whitespace in a block shouldn't prevent the user from terminating the code block #119355

Merged
merged 8 commits into from
May 22, 2024
Merged

gh-118911: Trailing whitespace in a block shouldn't prevent the user from terminating the code block #119355

merged 8 commits into from
May 22, 2024

Conversation

aelsayed95
Copy link
Contributor

@aelsayed95 aelsayed95 commented May 21, 2024 •
edited by bedevere-app bot
Loading

Issue described in #118911

Fix includes:

  1. If the user hits enter twice, they are able to exit out of the block, even if there's trailing whitespace.
  2. Trim trailing whitespace in the HistoricalReader to match IPython's behavior

@cpython-cla-bot CPython CLA Bot
Copy link

cpython-cla-bot bot commented May 21, 2024 •
edited
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented May 21, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app bedevere-app bot mentioned this pull request May 21, 2024
Closed
@lysnikolaou lysnikolaou added the topic-repl Related to the interactive shell label May 21, 2024
@aelsayed95
Copy link
Contributor Author

One issue discovered while testing this (exists on main too):

Within a code block, if the user hits enter twice to add new lines, the first line matches the previous line's indentation while the next one doesn't.
This doesn't match IPython's behaviour.

@aelsayed95
Copy link
Contributor Author

Added #119356

@lysnikolaou lysnikolaou added the needs backport to 3.13 bugs and security fixes label May 21, 2024
savannahostrowski
savannahostrowski reviewed May 21, 2024
View reviewed changes
Copy link
Member

@savannahostrowski savannahostrowski left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice! I tested this on Fedora (via Devcontainer) and it looks like this now works as described.

Misc/NEWS.d/next/Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst Outdated Show resolved Hide resolved
Lib/_pyrepl/readline.py Outdated Show resolved Hide resolved
ambv
ambv approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@ambv ambv left a comment

Choose a reason for hiding this comment

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

This looks good.

I merged new changes in main into your PR since there were conflicts, and reworded the blurb per my suggestion.

@ambv ambv merged commit 5091c44 into python:main May 22, 2024
43 checks passed
@miss-islington-app
Copy link

Thanks @aelsayed95 for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @aelsayed95 and @ambv, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 5091c4400c9ea2a2d1e4d89a28c9d0de2651fa6d 3.13

@ambv
Copy link
Contributor

ambv commented May 22, 2024

Thanks, Aya! ✨ 🍰 ✨

@ambv
Copy link
Contributor

ambv commented May 22, 2024

The backport depends on GH-119362. I'll deal with that tomorrow.

@lysnikolaou
Copy link
Member

Thanks, @aelsayed95! I've merged #119362. I'll open a backport PR now.

lysnikolaou pushed a commit to lysnikolaou/cpython that referenced this pull request May 22, 2024
@aelsayed95 @ambv @lysnikolaou
[3.13] pythongh-118911: Trailing whitespace in a block shouldn't prev…
13c31f1 
…ent the user from terminating the code block (pythonGH-119355)

(cherry picked from commit 5091c44)

Co-authored-by: Aya Elsayed <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented May 22, 2024

GH-119404 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label May 22, 2024
lysnikolaou added a commit that referenced this pull request May 23, 2024
@lysnikolaou @aelsayed95 @ambv
[3.13] gh-118911: Trailing whitespace in a block shouldn't prevent th…
9fa1b4f 
…e user from terminating the code block (GH-119355) (#119404)

(cherry picked from commit 5091c44)

Co-authored-by: Aya Elsayed <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
@JelleZijlstra JelleZijlstra mentioned this pull request May 28, 2024
Closed
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@aelsayed95 @ambv @estyxx
pythongh-118911: Trailing whitespace in a block shouldn't prevent the…
0d1ccbb 
… user from terminating the code block (python#119355)

Co-authored-by: Łukasz Langa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@savannahostrowski savannahostrowski savannahostrowski left review comments

@ambv ambv ambv approved these changes

Assignees

@ambv ambv

Labels
topic-repl Related to the interactive shell
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aelsayed95 @ambv @lysnikolaou @savannahostrowski