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

gh-129598: allow multi stmts for ast single with ';' #129620

Merged
merged 12 commits into from
Mar 19, 2025

Conversation

tom-pytel
Copy link
Contributor

@tom-pytel tom-pytel commented Feb 3, 2025

Had to add a bit more than just a visitor node due to possibility of block statements, so the following are all handled correctly:

$ ./python
Python 3.14.0a4+ (heads/astsingle:9ba281d871, Feb  3 2025, 09:15:49) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from ast import *

>>> print(unparse(parse('i = 1; "expr"; raise Exception', mode='exec')))
i = 1
'expr'
raise Exception

>>> print(unparse(parse('i = 1; "expr"; raise Exception', mode='single')))
i = 1; 'expr'; raise Exception

>>> print(unparse(parse('if i:\n "expr"\nelse:\n raise Exception', mode='single')))
if i:
    'expr'
else:
    raise Exception

>>> print(unparse(parse('@decorator\ndef func():\n "docstring"\n i = 1; "expr"; raise Exception', mode='single')))
@decorator
def func():
    """docstring"""
    i = 1
    'expr'
    raise Exception

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

Can we have tests please?

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

We might want to address the cleanup parts in a separate PR as it's a bit orthogonal. If possible I'd like @JelleZijlstra's opinion on whether we should worry about the state of the visitor after an error or not, and whether we should worry about whether it's reusable or not.

try:
self._write_docstring_and_traverse_body(node)
finally:
self._type_ignores.clear()
Copy link
Member

Choose a reason for hiding this comment

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

Another question is whether the mapping precedences should be cleared in visit() as well.

@tom-pytel
Copy link
Contributor Author

whether we should worry about whether it's reusable or not.

I don't think so. Its not exported from ast module, recreated on every call to unparse() and its not even really a class, just a wrapper around some very simple state.

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

Except for my question on the precedences mapping, I think it's fine.

@tom-pytel
Copy link
Contributor Author

tom-pytel commented Mar 1, 2025

Except for my question on the precedences mapping, I think it's fine.

Sorry, missed that, what is the question?

Ah, NM, its up there. Probably for someone else do decide and another PR?

@JelleZijlstra JelleZijlstra merged commit a8cb5e4 into python:main Mar 19, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants