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

bpo-38964: Print correct filename on a SyntaxError in an fstring #20399

Merged
merged 5 commits into from
May 26, 2020
Merged

bpo-38964: Print correct filename on a SyntaxError in an fstring #20399

merged 5 commits into from
May 26, 2020

Conversation

lysnikolaou
Copy link
Contributor

@lysnikolaou lysnikolaou commented May 25, 2020 •
edited by bedevere-bot
Loading

When a SyntaxError in the expression part of an fstring is found,
the filename attribute of the SyntaxError is always <fstring>.
With this PR it gets changed to always have the name of the file
the fstring resides in.

https://bugs.python.org/issue38964

@lysnikolaou
bpo-38964: Print correct filename on a SyntaxError in an fstring
e0a2f53 
When a `SyntaxError` in the expression part of an fstring is found,
the filename attribute of the `SyntaxError` is always `<fstring>`.
With this PR it gets changed to always have the name of the file
the fstring resides in.
@lysnikolaou
Copy link
Contributor Author

Can this be backported to 3.9?

@pablogsal
Copy link
Member

Can this be backported to 3.9?

The test would need a skip for the old parser if we backport this to 3.9

ericvsmith
ericvsmith reviewed May 25, 2020
View reviewed changes
Copy link
Member

@ericvsmith ericvsmith 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 to me. Thanks for fixing this, @lysnikolaou!

I have one tiny nit mentioned below, which you can ignore if you don't think it's important.

I think this should be backported to 3.9. It's clearly a bug.

Lib/test/test_fstring.py Outdated Show resolved Hide resolved
pablogsal
pablogsal reviewed May 25, 2020
View reviewed changes
Lib/test/test_fstring.py Outdated Show resolved Hide resolved
@pablogsal pablogsal added the needs backport to 3.9 only security fixes label May 25, 2020
@pablogsal pablogsal merged commit f7b1e46 into python:master May 26, 2020
@miss-islington
Copy link
Contributor

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

@miss-islington
Copy link
Contributor

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

@pablogsal
Copy link
Member

@lysnikolaou Can you do the backport using cherry_picker?

@lysnikolaou
Copy link
Contributor Author

On it.

lysnikolaou added a commit to lysnikolaou/cpython that referenced this pull request May 26, 2020
@lysnikolaou
[3.9] bpo-38964: Print correct filename on a SyntaxError in an fstring (
e42b7ba 
pythonGH-20399)

When a `SyntaxError` in the expression part of a fstring is found,
the filename attribute of the `SyntaxError` is always `<fstring>`.
With this commit, it gets changed to always have the name of the file
the fstring resides in.

Co-authored-by: Pablo Galindo <[email protected]>.
(cherry picked from commit f7b1e46)

Co-authored-by: Lysandros Nikolaou <[email protected]>
@bedevere-bot
Copy link

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

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label May 26, 2020
@lysnikolaou lysnikolaou deleted the filename-fstring-syntaxerrors branch May 26, 2020 00:42
pablogsal pushed a commit that referenced this pull request May 26, 2020
@lysnikolaou
[3.9] bpo-38964: Print correct filename on a SyntaxError in an fstring (
791a46e 
GH-20399) (GH-20404)

When a `SyntaxError` in the expression part of a fstring is found,
the filename attribute of the `SyntaxError` is always `<fstring>`.
With this commit, it gets changed to always have the name of the file
the fstring resides in.

Co-authored-by: Pablo Galindo <[email protected]>.
(cherry picked from commit f7b1e46)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericvsmith ericvsmith ericvsmith left review comments

@pablogsal pablogsal pablogsal approved these changes

Assignees

@pablogsal pablogsal

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@lysnikolaou @pablogsal @miss-islington @bedevere-bot @ericvsmith @the-knights-who-say-ni