fix: transform inputs with dedent#14822
Merged
Carreau merged 2 commits intoipython:mainfrom Mar 7, 2025
Merged
Conversation
Member
|
Thanks, this is quite old code, that dates back from readlines, when you could only get lines one by one, let's try it as is, and see if there are any other issues, in which case we'll have to add more tests. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes: #14818
The
leading_indentfunction in IPython/core/inputtransformer2.py had an issue in that it checked to see if the first line had a comment anywhere in it, and if it did, then it just would ignore checking if the rest of the input could possibly need to have indentation removed.The main issue is that it looks like the function assumes you can just check the first line, but that is not the case. You need to check the indentation level of every line in the input and remove the minimum (in terms of length) indentation common to all lines.
I'm not sure why this function isn't just using
textwrap.dedent, which seems like it does exactly what we want to be doing. I just switched the function to use this.For the tests, it turns out that there was a test cases that wasn't even being checked. I fixed that as well and added the sample I noted in #14818 as a 3rd test case.
While working on this, I wrote a more involved test, which didn't really fit with the style of the other tests, but I'll put it here in case you want to include it.
Details