GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-121804: always show error location for SyntaxError's in new repl by skirpichev · Pull Request #121886 · python/cpython

Conversation

skirpichev commented Jul 17, 2024 •

edited

Loading

Uh oh!

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

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

Issue: The new repl sometimes does not show the error location when SyntaxError is raised #121804

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

Sorry, something went wrong.

Uh oh!

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

All reactions


          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

skirpichev requested review from ambv, lysnikolaou, pablogsal and terryjreedy as code owners

July 17, 2024 03:45

bedevere-app Bot mentioned this pull request

Jul 17, 2024

The new repl sometimes does not show the error location when SyntaxError is raised #121804

Closed

bedevere-app Bot added the awaiting review label

Jul 17, 2024


          amend news

25dfe26

terryjreedy reviewed

Jul 17, 2024

View reviewed changes

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

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.

Sorry, something went wrong.

Uh oh!

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

All reactions

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.

All reactions

Sorry, something went wrong.

Uh oh!

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

skirpichev requested a review from terryjreedy

July 31, 2024 05:51

skirpichev added 3 commits

August 18, 2024 16:43


          Merge branch 'master' into show-syntaxerr-location-121804

f0e8d21


          Merge branch 'master' into show-syntaxerr-location-121804

a196862


          + reorder guard statements

1aeabf2

pablogsal reviewed

Aug 19, 2024

View reviewed changes

Comment thread

Lib/code.py Resolved

Uh oh!

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

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

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.

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

Sorry, something went wrong.

Uh oh!

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

All reactions

skirpichev Aug 19, 2024

Copy link

Copy Markdown

Member Author

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.

Maybe I should make it underscore-prefixed?

Sorry, something went wrong.

Uh oh!

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

All reactions

pablogsal Aug 19, 2024

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.

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.

Sorry, something went wrong.

Uh oh!

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

All reactions

skirpichev Aug 19, 2024

Copy link

Copy Markdown

Member Author

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.

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)

Sorry, something went wrong.

Uh oh!

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

All reactions

skirpichev added 2 commits

August 19, 2024 15:35


          Merge branch 'master' into show-syntaxerr-location-121804

9b3ff96


          address review: add guard and rewrote news entry

bb89ab1

skirpichev requested a review from pablogsal

August 19, 2024 12:56

pablogsal approved these changes

Aug 19, 2024

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting review labels

Aug 19, 2024

pablogsal added the needs backport to 3.13 only security fixes label

Aug 19, 2024

pablogsal merged commit 354d55e into python:main

Aug 19, 2024

bedevere-app Bot removed the awaiting merge label

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.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

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

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

All reactions

Sorry, something went wrong.

Uh oh!

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

miss-islington-app Bot assigned pablogsal

Aug 19, 2024

pablogsal commented Aug 19, 2024

Copy link

Copy Markdown

Member

Thanks a lot for your contribution @skirpichev 🚀

All reactions

Sorry, something went wrong.

Uh oh!

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

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?

All reactions

Sorry, something went wrong.

Uh oh!

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

skirpichev commented Aug 19, 2024 via email

Copy link

Copy Markdown

Member Author

On Mon, Aug 19, 2024 at 07:20:21AM -0700, Pablo Galindo Salgado wrote: Sorry, ***@***.*** and ***@***.***, I could not cleanly backport this to 3.13 due to a conflict. Please backport using [3]cherry_picker on command line. cherry_picker 354d55e 3.13 Seems the backport failed, do you mind following these instructions to open the 3.13 backport?

Yes, I'll work on this if you think that issue worth fixing for 3.13.

All reactions

Sorry, something went wrong.

Uh oh!

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

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 <[email protected]>

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.

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-app Bot removed the needs backport to 3.13 only 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.

All reactions

Sorry, something went wrong.

Uh oh!

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

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.

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

Uh oh!

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

Assignees

pablogsal

Labels

None yet

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.

3 participants

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.

GeezerVison

Conversation

skirpichev commented Jul 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

skirpichev commented Jul 17, 2024

Uh oh!

Uh oh!

pablogsal Aug 19, 2024

Choose a reason for hiding this comment

Uh oh!

skirpichev Aug 19, 2024

Choose a reason for hiding this comment

Uh oh!

pablogsal Aug 19, 2024

Choose a reason for hiding this comment

Uh oh!

skirpichev Aug 19, 2024

Choose a reason for hiding this comment

Uh oh!

miss-islington-app Bot commented Aug 19, 2024

Uh oh!

miss-islington-app Bot commented Aug 19, 2024

Uh oh!

pablogsal commented Aug 19, 2024

Uh oh!

pablogsal commented Aug 19, 2024

Uh oh!

skirpichev commented Aug 19, 2024 via email

Uh oh!

bedevere-app Bot commented Aug 19, 2024

Uh oh!

bedevere-app Bot commented Aug 26, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

skirpichev commented Jul 17, 2024 •

edited

Loading

Uh oh!

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