Skip to content

Fix IndentationError works differently with cpython in interective shell #4399

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

Merged
merged 4 commits into from
Jan 2, 2023

Conversation

branai
Copy link
Contributor

@branai branai commented Jan 2, 2023

Fixes #3892 #4400

Removed the automatic continuing logic that was in place for code that needs multiple lines in the shell to parse correctly. Now we check if full_code can be parsed after each line of input given, but ignoring specific EOF and IndentationLevel errors.

I am working on a way to do this without modifying errors.rs, if I should not be modifying that.

Edit: I think my edits to errors.rs may have improved EOF error handling.

Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

Thank you!

@youknowone youknowone merged commit d0f6af0 into RustPython:main Jan 2, 2023
@DimitrisJim
Copy link
Member

I think this PR might have been a bit more complex than needed. As far as I can tell, the main issue here could of been solved by just tweaking the ShellExecResult::Continue case to check if an empty line was the input (and only an empty line).

There's also a couple of little issues here, the Indentation Error isn't exactly the right one and now any line containing a comment or space/tabs results in an indent error.

The changes to error seem right though, but it can also be slightly tweaked to simplify (matching against it in shell_exec as the others do. For this we probably could use an IndentError with a wrong token value that we only use for the match.

@branai
Copy link
Contributor Author

branai commented Jan 3, 2023

@DimitrisJim I agree this fix was too complex, I simplified it in #4406 . The improved fix does exactly what you said with checking for an empty line. There are also cases in continue mode when a non-empty line is given where an error should be thrown, so I accounted for those too.

I'll look into what you mentioned about the indentationerror

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.

IndentationError works differently with cpython in interective shell.
3 participants