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-36817: Add f-string debugging using '='. #13123

Merged
merged 53 commits into from
May 8, 2019
Merged

bpo-36817: Add f-string debugging using '='. #13123

merged 53 commits into from
May 8, 2019

Conversation

ericvsmith
Copy link
Member

@ericvsmith ericvsmith commented May 6, 2019 •
edited by bedevere-bot
Loading

@ericvsmith
WIP: Mostly works, except repr isn't called (so only string expressio…
6797767 
…ns currently work).
@ericvsmith
WIP: Change !x to !d, to match an earlier discussion on python-ideas.
93df017
@ericvsmith
Call repr on the resulting expression. Change expr_source to expr_text.
99283dc
@ericvsmith
Add simple tests.
1b29503
@ericvsmith
Remove FVC_DEBUG, which I thought I was going to use for this feature…
2684897 
…, but it ended up not being needed.
@ericvsmith
Fix some comments and formatting.
42582d5
@ericvsmith
Added blurb.
1ffa2a1
@ericvsmith
Use quotes around the string result in the blurb text.
14af246
@ericvsmith
Call out the conversion specifiers in monospace.
f985b10
@ericvsmith
Note that the repr of the expression is used.
4556421
@ericvsmith
Fix conversion character.
856485a
@ericvsmith
Added Whats New entry.
a384516
@ericvsmith
Don't allow format_spec with !d.
6107b54
@ericvsmith
Test that we don't allow a format spec.
3f292c5
@ericvsmith
Use repr(expr) if no format spec, use format(expr, spec) if one is.
0046262
@ericvsmith
Pre-append the '=' to the end of expr_text at compile time, instead o…
a697d02 
…f adding it at runtime.
@ericvsmith
Add a coding comment, a note about why I'm using a unicode identifier…
49d03e4 
…, and a test with a unicode literal.
@ericvsmith
Added tests for nested f-strings and for pre- and post-text.
f79e21d
@ericvsmith
Add a test for newlines in expressions.
ed82083
@ericvsmith
Clarify a comment.
aeef3db
@ericvsmith
Change from di to =.
caf3221
@ericvsmith
Fix a comment.
921d8a1
@ericvsmith
Fix comment.
bd65ec1
@ericvsmith
Fix comments.
1acc386
@ericvsmith
Skip all whitespace after the =, not just spaces. Handle unparsing.
d46f83c
@ericvsmith
Hack: Modify tests for f-string annotations. I'm not sure these are e…
abff2cb 
…ven valid tests any more.
@ericvsmith
Add missing FVC_ASCII.
75ef1ac
@ericvsmith
Move the mapping from "no converion specified" to the exact conversion
e688fbf 
function in to the compile phase.
@ericvsmith
Make ast_unparse understand =.
3e7e9ff
@ericvsmith
Add a test for !s.
6a39185
@larryhastings
Copy link
Contributor

The two failed tests both failed in the same way: Lib/test/test_fstring.py failed a nanny check for whitespace. "make patchcheck" will rub your nose in it.

@ericvsmith
Copy link
Member Author

I fixed the whitespace problem. I kept fighting emacs over that, and couldn't figure out what problem was. Thanks, Larry and patchcheck.

I'm not sure which tests you're referencing that are failing. All of the tests pass for me on macOS.

larryhastings
larryhastings reviewed May 7, 2019
View reviewed changes
Copy link
Contributor

@larryhastings larryhastings left a comment

Choose a reason for hiding this comment

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

Minor nonsense, mostly about comments. I didn't mark this as "Approve" just because I wanted to see what you had to say in reply; if you tell me it's all fine then I'll probably just Approve it then.

Lib/test/test_fstring.py Outdated Show resolved Hide resolved
Lib/test/test_fstring.py Outdated Show resolved Hide resolved
Misc/NEWS.d/next/Core and Builtins/2019-05-02-11-48-08.bpo-36774.ZqbJ1J.rst Outdated Show resolved Hide resolved
Misc/NEWS.d/next/Core and Builtins/2019-05-02-11-48-08.bpo-36774.ZqbJ1J.rst Outdated Show resolved Hide resolved
Python/ast.c Outdated Show resolved Hide resolved
Python/ast.c Outdated Show resolved Hide resolved
Python/ast.c Show resolved Hide resolved
Python/ast_unparse.c Outdated Show resolved Hide resolved
Python/compile.c Outdated Show resolved Hide resolved
@asottile asottile mentioned this pull request May 7, 2019
Closed
@ericvsmith ericvsmith self-assigned this May 8, 2019
@larryhastings
Copy link
Contributor

The test I was referring to were CI tests. Azure and something else were both complaining about the whitespace, and with your latest update now they're not.

larryhastings
larryhastings requested changes May 8, 2019
View reviewed changes
Copy link
Contributor

@larryhastings larryhastings left a comment

Choose a reason for hiding this comment

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

OH WHY DO PEOPLE COMPLAIN ABOUT THIS PIDDLING LITTLE STUFF

MY LIFE

Lib/test/test_fstring.py Outdated Show resolved Hide resolved
Python/ast.c Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@ericvsmith
Copy link
Member Author

I have made the requested changes; please review again.

The CI errors are related to cert auth failures, They're unrelated to this change.

@bedevere-bot
Copy link

Thanks for making the requested changes!

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

@ericvsmith ericvsmith closed this May 8, 2019
@ericvsmith ericvsmith reopened this May 8, 2019
@gpshead @ericvsmith
bpo-36816: Update the self-signed.pythontest.net cert (pythonGH-13192)
19dc0d0 
We updated the server, our testsuite must match.


https://bugs.python.org/issue36816


✈️ CLE -> DEN ✈️ #pycon2019

(cherry picked from commit 6bd8173)
@ericvsmith ericvsmith merged commit 9a4135e into python:master May 8, 2019
@ericvsmith ericvsmith deleted the fstring-equal branch May 8, 2019 20:34
@asottile asottile mentioned this pull request May 10, 2019
Closed
@gousaiyang gousaiyang mentioned this pull request Jan 16, 2020
Closed
@ericvsmith ericvsmith mentioned this pull request Apr 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments

@tirkarthi tirkarthi tirkarthi left review comments

@larryhastings larryhastings larryhastings approved these changes

Assignees

@ericvsmith ericvsmith

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ericvsmith @larryhastings @bedevere-bot @serhiy-storchaka @tirkarthi @the-knights-who-say-ni @gpshead