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.

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

bpo-34463: Match python traceback format to the C traceback for a Syn… #23427

Merged
merged 4 commits into from Dec 22, 2020

Conversation

iritkatriel
Copy link
Member

@iritkatriel iritkatriel commented Nov 20, 2020

…taxError without lineno.

https://bugs.python.org/issue34463

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Dec 21, 2020
Copy link
Contributor

@taleinat taleinat left a comment

Choose a reason for hiding this comment

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

Just one minor comment.

Also, this may be a good opportunity to convert this code to use f-strings; ISTM it would make it slightly clearer.

Lib/traceback.py Outdated
Comment on lines 589 to 594
filename = self.filename or "<string>"
lineno = str(self.lineno) or '?'
yield ' File "{}", line {}\n'.format(filename, lineno)
filename_suffix = ''
if self.lineno is not None:
yield ' File "{}", line {}\n'.format(filename, self.lineno)
elif self.filename is not None:
filename_suffix = ' ({})'.format(self.filename)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since filename is only used in the first "if" block, but self.filename is used later, I'd move its definition into the block, or just remove it and use self.filename or "<string>" inline.

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@iritkatriel
Copy link
Member Author

Indeed. I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@taleinat: please review the changes made to this pull request.

@taleinat taleinat added needs backport to 3.8 only security fixes needs backport to 3.9 only security fixes type-bug An unexpected behavior, bug, or error and removed stale Stale PR or inactive for long period of time. labels Dec 22, 2020
@taleinat taleinat merged commit 069560b into python:master Dec 22, 2020
@miss-islington
Copy link
Contributor

Thanks @iritkatriel for the PR, and @taleinat for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 22, 2020
…xErrors without a lineno (pythonGH-23427)

(cherry picked from commit 069560b)

Co-authored-by: Irit Katriel <iritkatriel@yahoo.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Dec 22, 2020
@bedevere-bot
Copy link

GH-23895 is a backport of this pull request to the 3.9 branch.

@miss-islington
Copy link
Contributor

Sorry, @iritkatriel and @taleinat, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 069560b1171eb6385121ff3b6331e8814a4e7454 3.8

@iritkatriel
Copy link
Member Author

@taleinat Thanks, I'll sort out the backport to 3.8

miss-islington added a commit that referenced this pull request Dec 22, 2020
…xErrors without a lineno (GH-23427)

(cherry picked from commit 069560b)

Co-authored-by: Irit Katriel <iritkatriel@yahoo.com>
iritkatriel added a commit to iritkatriel/cpython that referenced this pull request Dec 22, 2020
SyntaxErrors without a lineno (pythonGH-23427)

(cherry picked from commit 069560b)

Co-authored-by: Irit Katriel <iritkatriel@yahoo.com>
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
ambv pushed a commit that referenced this pull request Apr 26, 2021
…#23899)

* [3.8] bpo-34463: Make python tracebacks identical to C tracebacks for
SyntaxErrors without a lineno (GH-23427)

(cherry picked from commit 069560b)

Co-authored-by: Irit Katriel <iritkatriel@yahoo.com>

* 📜🤖 Added by blurb_it.

* added missing newline in test

Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
@iritkatriel iritkatriel deleted the bpo-34463 branch August 2, 2021 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs backport to 3.8 only security fixes type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants