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-113744: Add a new IncompleteInputError exception to improve incomplete input detection in the codeop module #113745

Merged
merged 2 commits into from
Jan 30, 2024

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Jan 5, 2024

Copy link
Member

@lysnikolaou lysnikolaou left a comment

Choose a reason for hiding this comment

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

LGTM! Something needs to be regenerated.

…incomplete input detection in the codeop module

Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
@pablogsal pablogsal enabled auto-merge (squash) January 30, 2024 15:04
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM.

I am not sure about the name. On one hand, it looks correct and I do not have anything better, on other hand, it can be confused with the use case for BlockingIOError. In future we can see misuse of this error in cases not related to parsing Python sources. Not sure if it's something to worry about.

@pablogsal
Copy link
Member Author

I am not sure about the name. On one hand, it looks correct and I do not have anything better, on other hand, it can be confused with the use case for BlockingIOError. In future we can see misuse of this error in cases not related to parsing Python sources. Not sure if it's something to worry about.

I don't think we should worry about it because it's not user facing (at least for now). It's used internally in a special mode of the parser that's also not user facing. And in the rare case where a user encounters it it's relatively easy to identify why is appearing.

…mprove incomplete input detection in the codeop module

Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
@encukou
Copy link
Member

encukou commented Jan 30, 2024

Let's not add it to the stable ABI, then?

@pablogsal pablogsal merged commit 39d102c into python:main Jan 30, 2024
@pablogsal pablogsal deleted the incomplete branch January 30, 2024 16:21
@pablogsal
Copy link
Member Author

Let's not add it to the stable ABI, then?

Sorry @encukou but seems that the auto-land landed it before I could tackle your comment. Do you want to remove it from the stable ABI? The name is public in any case so it would be weird if is not there but you can invoke it from builtins

@serhiy-storchaka
Copy link
Member

If it is used internally in a special mode of the parser, it could be not added to buildins. But I do not see how this mode is special. It does not use any magic flags or private arguments. I expect it to be used in many third-party REPL implementations. And maybe even in the C code. So it is okay to make it as public as other SyntaxError subclasses.

@encukou
Copy link
Member

encukou commented Jan 31, 2024

I don't think we should worry about [the name] because it's not user facing (at least for now).

Adding it to the public C API (and especially to the stable ABI) means that it is user facing, and now is the time to worry about the name.
If there is a chance we'll find a better way, and we'll need to deprecate/remove this, it shouldn't go it the stable ABI.

TBH, the name is fine with me. IncompleteSourceError might be more descriptive.

aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
…incomplete input detection in the codeop module (python#113745)

Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants