Use non-parenthesized range for DebugText#9953
Merged
dhruvmanila merged 1 commit intomainfrom Feb 12, 2024
Merged
Conversation
Member
|
Can you add a short explanation why it should use the expression range instead of the parenthesized range? |
Contributor
|
MichaReiser
approved these changes
Feb 12, 2024
Member
MichaReiser
left a comment
There was a problem hiding this comment.
Thanks for the explanation. It helped me understand the change and makes this a very easy review :)
DebugText with non-parenthesized rangeDebugText
nkxxll
pushed a commit
to nkxxll/ruff
that referenced
this pull request
Mar 10, 2024
## Summary
This PR fixes the `DebugText` implementation to use the expression range
instead of the parenthesized range.
Taking the following code snippet as an example:
```python
x = 1
print(f"{ ( x ) = }")
```
The output of running it would be:
```
( x ) = 1
```
Notice that the whitespace between the parentheses and the expression is
preserved as is.
Currently, we don't preserve this information in the AST which defeats
the purpose of `DebugText` as the main purpose of the struct is to
preserve whitespaces _around_ the expression.
This is also problematic when generating the code from the AST node as
then the generator has no information about the parentheses the
whitespaces between them and the expression which would lead to the
removal of the parentheses in the generated code.
I noticed this while working on the f-string formatting where the debug
text would be used to preserve the text surrounding the expression in
the presence of debug expression. The parentheses were being dropped
then which made me realize that the problem is instead in the parser.
## Test Plan
1. Add a test case for the parser
2. Add a test case for the generator
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.
Summary
This PR fixes the
DebugTextimplementation to use the expression range instead of the parenthesized range.Taking the following code snippet as an example:
The output of running it would be:
Notice that the whitespace between the parentheses and the expression is preserved as is.
Currently, we don't preserve this information in the AST which defeats the purpose of
DebugTextas the main purpose of the struct is to preserve whitespaces around the expression.This is also problematic when generating the code from the AST node as then the generator has no information about the parentheses the whitespaces between them and the expression which would lead to the removal of the parentheses in the generated code.
I noticed this while working on the f-string formatting where the debug text would be used to preserve the text surrounding the expression in the presence of debug expression. The parentheses were being dropped then which made me realize that the problem is instead in the parser.
Test Plan