FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

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

/ cpython Public

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
ConversationCommits (7)ChecksFiles changed
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 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

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 Bot mentioned this pull request Jul 17, 2024
Closed
terryjreedy reviewed Jul 17, 2024
View reviewed changes

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 commented Jul 17, 2024

Copy link
Copy Markdown
Member Author

I presume that this will be backported to 3.13.

I'm not sure that referenced issue is important enough to do backport. The #121804 is not specific for the new repl. It's valid for the basic repl as well, long before 3.12.

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

I doubt it does make sense. Support for kwargs was silently added for showsyntaxerror() in #119318, it's not backported to 3.12. But in 3.13+, probably subclasses of InteractiveInterpreter should be adjusted to accept kwargs.

This patch doesn't fix anything for IDLE, apart from this small incompatibility in interfaces. The referenced issue isn't valid for the IDLE - here always correct error locations are highlighted.

skirpichev requested a review from terryjreedy July 31, 2024 05:51
pablogsal reviewed Aug 19, 2024
View reviewed changes
Comment thread Lib/code.py Outdated
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 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Maybe I should make it underscore-prefixed?

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 requested a review from pablogsal August 19, 2024 12:56
pablogsal added the needs backport to 3.13 bugs and security fixes label Aug 19, 2024
pablogsal merged commit 354d55e into python:main Aug 19, 2024

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 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 commented Aug 19, 2024

Copy link
Copy Markdown
Member

Thanks a lot for your contribution @skirpichev 🚀

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 commented Aug 19, 2024 via email

Copy link
Copy Markdown
Member Author

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
[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 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 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
[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
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
pythongh-121804: Always show error location for SyntaxError's in new …
737a58a 
…repl (python#121886)

bedevere-app Bot commented Aug 26, 2024

Copy link
Copy Markdown

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

terryjreedy added a commit that referenced this pull request Aug 26, 2024
[3.12] gh-121804: Backport idlelib.pyshell change (GH-121886) (#123366)
4deb32a 
To aid future backports here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Assignees

pablogsal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL