Remove unnecessary f-string handling in comment placement #20580
Conversation
|
|
It's very likely that this code is unused but it might be for a different reason than outlined in the PR and I'd prefer to fully understand why before removing this code.
Can you explain what this new logic is? Reading #6898. My impression is that the issue is that the To me, it also seems that the comment is still true because the interpolated element's range only covers |
| // These can't be parenthesized, as they must fall between two string tokens in an implicit | ||
| // concatenation. But the expression ranges only include the `1` and `2` above, so we also | ||
| // can't lex the contents between them. | ||
| if comment.enclosing_node().is_expr_f_string() { |
There was a problem hiding this comment.
I can't comment there but doesn't this change result in the SimpleTokenizer trying to lex ' # comment on line 136. This would be invalid because the SimpleTokenizer should never be pointed inside a string.
2 participants
In #7047 some logic was added to comment placement in order to address a panic in #6898 . I believe that panic was caused by old behavior of
SimpleTokenizerthat is no longer present, because it does not occur if the new logic is simply removed.This was discovered while trying to figure out the potential side-effects of #20578 - I originally was going to augment the logic to include t-strings here, but discovered that you could instead just remove it (unless I missed something?)