gh-121804: always show error location for SyntaxError's in new repl by skirpichev · Pull Request #121886 · python/cpython · GitHub
Skip to content

gh-121804: always show error location for SyntaxError's in new repl - #121886

Merged
pablogsal merged 7 commits into
python:mainfrom
skirpichev:show-syntaxerr-location-121804
Aug 19, 2024
Merged

gh-121804: always show error location for SyntaxError's in new repl#121886
pablogsal merged 7 commits into
python:mainfrom
skirpichev:show-syntaxerr-location-121804

Conversation

@skirpichev

@skirpichev skirpichev commented Jul 17, 2024 •
edited
Loading

Copy link
Copy Markdown
Member 
>>> def good(x, y): ...
... def bad(x, x): ...
  File "<python-input-13>", line 2
    def bad(x, x): ...
               ^
SyntaxError: duplicate argument 'x' in function definition

Notes:

  • showsyntaxerror() has undocumented colorize kwarg; new source argument also left undocumented
  • similar fix is possible for basic repl (see issue thread, here); if that does make sense - I'll provide a separate pr

@skirpichev
pythongh-121804: always show error location for SyntaxError's in new …
f5f0d53 
…repl

>>> def good(x, y): ...
... def bad(x, x): ...
  File "<python-input-13>", line 2
    def bad(x, x): ...
               ^
SyntaxError: duplicate argument 'x' in function definition
@bedevere-app bedevere-app Bot mentioned this pull request Jul 17, 2024
Closed
terryjreedy
terryjreedy reviewed Jul 17, 2024
View reviewed changes

@terryjreedy terryjreedy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I presume that this will be backported to 3.13. I would like the pyshell addition backported to 3.12 since I showsyntaxerror needs revision. If you don't, I will.

@skirpichev

skirpichev commented Jul 17, 2024

Copy link
Copy Markdown
Member Author

@skirpichev
skirpichev requested a review from terryjreedy July 31, 2024 05:51
pablogsal
pablogsal reviewed Aug 19, 2024
View reviewed changes
Comment thread Lib/code.py Outdated
pablogsal
pablogsal reviewed Aug 19, 2024
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2024-07-16-20-49-07.gh-issue-121804.gYN-In.rst Outdated
@@ -0,0 +1,3 @@
Add new kwarg ``source`` for

@pablogsal pablogsal Aug 19, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't want to make this argument public so let's not mention it in the change log . Instead, mention what we are fixing in the repl

@skirpichev skirpichev Aug 19, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Maybe I should make it underscore-prefixed?

@pablogsal pablogsal Aug 19, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Our experience is that if it appears in the signature, people will use it and then changing it will be more challenging. Also some times the underscore prefixed argument is used to avoid collision with keywords or other reserved identifiers.

@skirpichev skirpichev Aug 19, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Our experience is that if it appears in the signature, people will use it and then changing it will be more challenging.

Yeah, I did a proposal to hide this per default: https://discuss.python.org/t/25543.

Also some times the underscore prefixed argument is used to avoid collision with keywords

I was thinking that it's pep8 violation. "it is generally better to append a single trailing underscore rather than use an abbreviation or spelling corruption." (c)

@skirpichev
skirpichev requested a review from pablogsal August 19, 2024 12:56
@pablogsal pablogsal added the needs backport to 3.13 bugs and security fixes label Aug 19, 2024
@pablogsal
pablogsal merged commit 354d55e into python:main Aug 19, 2024
@miss-islington-app

miss-islington-app Bot commented Aug 19, 2024

Copy link
Copy Markdown

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

@miss-islington-app

miss-islington-app Bot commented Aug 19, 2024

Copy link
Copy Markdown

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

cherry_picker 354d55eb1fa40f272419aa6459ee5d2c4804c8ea 3.13

@pablogsal

pablogsal commented Aug 19, 2024

Copy link
Copy Markdown
Member

Thanks a lot for your contribution @skirpichev 🚀

@pablogsal

pablogsal commented Aug 19, 2024

Copy link
Copy Markdown
Member

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

cherry_picker 354d55eb1fa40f272419aa6459ee5d2c4804c8ea 3.13

Seems the backport failed, do you mind following these instructions to open the 3.13 backport?

@skirpichev

skirpichev commented Aug 19, 2024 via email

Copy link
Copy Markdown
Member Author

@skirpichev
skirpichev deleted the show-syntaxerr-location-121804 branch August 19, 2024 15:45
skirpichev added a commit to skirpichev/cpython that referenced this pull request Aug 19, 2024
@skirpichev
[3.13] pythongh-121804: Always show error location for SyntaxError's …
b8742c2 
…in new repl (pythonGH-121886)

(cherry picked from commit 354d55e)

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
@bedevere-app

bedevere-app Bot commented Aug 19, 2024

Copy link
Copy Markdown

GH-123148 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 Aug 19, 2024
pablogsal pushed a commit that referenced this pull request Aug 19, 2024
@skirpichev
[3.13] gh-121804: Always show error location for SyntaxError's in new…
c8f4069 
… repl (GH-121886) (#123148)

(cherry picked from commit 354d55e)
jeremyhylton pushed a commit to jeremyhylton/cpython that referenced this pull request Aug 19, 2024
@skirpichev @jeremyhylton
pythongh-121804: Always show error location for SyntaxError's in new …
cc9893b 
…repl (python#121886)
blhsing pushed a commit to blhsing/cpython that referenced this pull request Aug 22, 2024
@skirpichev @blhsing
pythongh-121804: Always show error location for SyntaxError's in new …
737a58a 
…repl (python#121886)
@bedevere-app

bedevere-app Bot commented Aug 26, 2024

Copy link
Copy Markdown

terryjreedy added a commit that referenced this pull request Aug 26, 2024
@terryjreedy
[3.12] gh-121804: Backport idlelib.pyshell change (GH-121886) (#123366)
4deb32a 
To aid future backports here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pablogsal pablogsal pablogsal approved these changes
@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
@ambv ambv Awaiting requested review from ambv ambv is a code owner
@terryjreedy terryjreedy Awaiting requested review from terryjreedy terryjreedy is a code owner

Assignees

@pablogsal pablogsal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@skirpichev @pablogsal @terryjreedy