gh-108469: Update ast.unparse for unescaped quote support from PEP701 [3.12] by tonybaloney · Pull Request #108553 · python/cpython

tonybaloney · 2023-08-27T22:29:16Z

Issue: ast.unparse doesn't observe the new PEP701 string delimiter rules #108469

sunmy2019 · 2023-09-02T20:28:06Z

Sorry that I have taken over.

tonybaloney · 2023-09-02T22:00:21Z

Sorry that I have taken over.

You're welcome. I spent hours debugging these private methods and can't figure out which ones need removing or changing

sunmy2019 · 2023-09-03T09:55:11Z

Oh, I've found some corner case that previous logic did not handle. Give me some time for a more thorough rewrite.

sunmy2019 · 2023-09-03T11:01:35Z

I discovered it was a bug in the original implementation. Opened #108843.

sunmy2019 · 2023-09-03T12:42:35Z

Fixed on this branch.

isidentical · 2023-09-03T17:44:28Z

 def visit_FormattedValue(self, node):
 def unparse_inner(inner):
- unparser = type(self)(_avoid_backslashes=True)
+ unparser = type(self)(_avoid_backslashes=False)


Do we still need this mode or can we drop avoid_backslashes all together?

I guess we can drop it.

isidentical · 2023-09-03T17:48:38Z

+ for c in value:
+ if c == "\\" or not c.isprintable():
+ res.append(c.encode("unicode_escape").decode("ascii"))
+ continue
+ if c == quote:
+ if consecutive_quotes == len(quote_type) - 1:
+ # escape when we see a full `quote_type`
+ res.append("\\")
+ consecutive_quotes = 0
+ else:
+ consecutive_quotes += 1
+ else:
+ consecutive_quotes = 0
+ res.append(c)


This logic probably belongs somewhere else, something similiar to what we had with the existing f-string unparsing implementation:
https://github.com/python/cpython/blob/21229ce1f0c8fb2e580df62972296d629960f30e/Lib/ast.py#L1182C1-L1218C1

isidentical · 2023-09-03T17:52:10Z

@sunmy2019 if we are interested in quickly shipping a fix for it, I'd avoid changing anything cosmetic and simply dropping the existing avoid_backslashes logic all together. It should probably probably work for all the supported cases. We can then simplify the handling of string literals with backslashes in a subsequent PR that only improves the generated code cosmeticly

sunmy2019 · 2023-09-03T18:30:56Z

I'd avoid changing anything cosmetic and simply dropping the existing avoid_backslashes logic all together

That what I tried to do before 1da1d4c.

Then I discovered a bug (#108843) in the original implementation. So I thought I can fix it along the way.

What do you think?

isidentical · 2023-09-04T15:34:40Z

I think separting the fix for that from the actual PEP 701 support makes sense, and also reduces the possible complexity of changes this PR brings in. Let's first do the cleanup and the support for PEP 701 and we can do the fix later since this seems like the more serious problem.

pablogsal · 2023-09-04T19:06:12Z

Let's try to land this today or tomorrow. I added @Yhg1s to the PR so he is aware of it.

This reverts commit 21229ce.

sunmy2019 · 2023-09-05T01:38:28Z

Let's first do the cleanup and the support for PEP 701

I got it reverted.

pablogsal · 2023-09-05T20:01:12Z

@sunmy2019 can you make a PR for the fix changes and ping me, @isidentical and @Yhg1s ?

miss-islington · 2023-09-05T20:02:06Z

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

…PEP701 [3.12] (pythonGH-108553) (cherry picked from commit 2c4c26c) Co-authored-by: Anthony Shaw <anthony.p.shaw@gmail.com> Co-authored-by: sunmy2019 <59365878+sunmy2019@users.noreply.github.com>

bedevere-bot · 2023-09-05T20:02:16Z

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

… PEP701 [3.12] (GH-108553) (#108960) Co-authored-by: Anthony Shaw <anthony.p.shaw@gmail.com> Co-authored-by: sunmy2019 <59365878+sunmy2019@users.noreply.github.com>

sunmy2019 · 2023-09-06T03:22:13Z

@sunmy2019 can you make a PR for the fix changes and ping me, @isidentical and @Yhg1s ?

Working on it.

As of python#108553, the `_avoid_backslashes` code path is dead `scape_newlines` was introduced in python#110271. Happy to drop the typo fix if we don't want it

As of #108553, the `_avoid_backslashes` code path is dead `scape_newlines` was introduced in #110271. Happy to drop the typo fix if we don't want it

As of python#108553, the `_avoid_backslashes` code path is dead `scape_newlines` was introduced in python#110271. Happy to drop the typo fix if we don't want it

Add two tests for ast.unparse for unescaped quote support from PEP701
5ee5d57

bedevere-bot mentioned this pull request Aug 27, 2023
ast.unparse doesn't observe the new PEP701 string delimiter rules #108469
Closed
2 tasks

bedevere-bot added the tests Tests in the Lib/test dir label Aug 27, 2023

tonybaloneyand others added 3 commits August 28, 2023 08:29

Merge branch 'main' into unparse_pep701
20b6455

Merge branch 'main' into unparse_pep701
d9bf98a

unparse with new fstring syntax
cb043e0

Add news
5ea8f4f

sunmy2019 marked this pull request as ready for review September 2, 2023 20:40

sunmy2019 requested review from isidentical, lysnikolaou and pablogsal as code owners September 2, 2023 20:40

bedevere-bot added the awaiting review label Sep 2, 2023

Merge branch 'main' into unparse_pep701
e716e03

sunmy2019 reviewed Sep 2, 2023
View reviewed changes

Comment threadLib/test/test_unparse.py Outdated

Update Lib/test/test_unparse.py
1da1d4c

simplify things
43d177c

sunmy2019 mentioned this pull request Sep 3, 2023
ast.unparse is not working correctly for strings with multiple quotes #108843
Closed

sunmy2019 added 3.12 only security fixes 3.13 bugs and security fixes and removed tests Tests in the Lib/test dir labels Sep 3, 2023

reimplement quote selection logic
21229ce

isidentical reviewed Sep 3, 2023
View reviewed changes

pablogsal requested a review from Yhg1s September 4, 2023 18:25

sunmy2019 added 2 commits September 5, 2023 09:22

Revert "reimplement quote selection logic"
9b1b5dd
This reverts commit 21229ce.

clean up unnecessary parts
88394d1

pablogsal approved these changes Sep 5, 2023
View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Sep 5, 2023

pablogsal merged commit 2c4c26c into python:mainSep 5, 2023

bedevere-bot removed the awaiting merge label Sep 5, 2023

pablogsal added the needs backport to 3.12 only security fixes label Sep 5, 2023

bedevere-bot removed the needs backport to 3.12 only security fixes label Sep 5, 2023

hauntsaninja mentioned this pull request Dec 31, 2023
Clean up backslash avoiding code in ast, fix typo #113605
Merged

hauntsaninja added a commit that referenced this pull request Jan 16, 2024
Clean up backslash avoiding code in ast, fix typo (#113605)
3d5df54
As of #108553, the `_avoid_backslashes` code path is dead `scape_newlines` was introduced in #110271. Happy to drop the typo fix if we don't want it

Uh oh!

Uh oh!

Conversation

tonybaloney commented Aug 27, 2023• edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sunmy2019 commented Sep 2, 2023

Uh oh!

Uh oh!

tonybaloney commented Sep 2, 2023

Uh oh!

sunmy2019 commented Sep 3, 2023

Uh oh!

sunmy2019 commented Sep 3, 2023

Uh oh!

sunmy2019 commented Sep 3, 2023

Uh oh!

isidenticalSep 3, 2023

Choose a reason for hiding this comment

Uh oh!

sunmy2019Sep 3, 2023

Choose a reason for hiding this comment

Uh oh!

isidenticalSep 3, 2023

Choose a reason for hiding this comment

Uh oh!

isidentical commented Sep 3, 2023

Uh oh!

sunmy2019 commented Sep 3, 2023

Uh oh!

isidentical commented Sep 4, 2023

Uh oh!

pablogsal commented Sep 4, 2023

Uh oh!

sunmy2019 commented Sep 5, 2023

Uh oh!

pablogsal commented Sep 5, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miss-islington commented Sep 5, 2023

Uh oh!

bedevere-bot commented Sep 5, 2023

Uh oh!

sunmy2019 commented Sep 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tonybaloney commented Aug 27, 2023•
edited by bedevere-bot
Loading

pablogsal commented Sep 5, 2023•
edited
Loading