Preserve backslash in raw string literal#6152
Conversation
PR Check ResultsBenchmarkLinuxWindows |
...ff_python_formatter/tests/snapshots/black_compatibility@miscellaneous__string_quotes.py.snap
Outdated
Show resolved
Hide resolved
| newlines = ContainsNewlines::Yes; | ||
| } else if !quotes.triple { | ||
| if c == '\\' { | ||
| if !is_raw && c == '\\' { |
There was a problem hiding this comment.
Do we need to move this check to line 490 instead because we need to make sure that quotes are properly escaped. Can you add the following test
r'It\'s normalizing \' and " quotes'This should be formatted as:
r"It's normalizing ' and \" quotes"rThere was a problem hiding this comment.
@MichaReiser r'It\'s normalizing \' and " quotes' is not equivalent to r"It's normalizing ' and \" quotes":
>>> r'It\'s normalizing \' and " quotes'
'It\\\'s normalizing \\\' and " quotes'
>>> r"It's normalizing ' and \" quotes"
'It\'s normalizing \' and \\" quotes'
>>> r'It\'s normalizing \' and " quotes' == r"It's normalizing ' and \" quotes"
FalseThere was a problem hiding this comment.
Okay, I now played with your PR and found an example that produces invalid syntax:
# Input
r'Not-so-tricky "quote \'\''
# Ruff
r"Not-so-tricky "quote \'\'"
# Black
r'Not-so-tricky "quote \'\''Note how Ruff changes the quotes from ' to " but fails to escape the ".
We need to play a bit more with black to understand how black determines the preferred quotes for raw strings, and how the normalization has to work. Maybe @konstin knows more, because I'm not that familiar with Python and I must say, the escaping logic behind raw strings is confusing to me (you have to escape quotes)
There was a problem hiding this comment.
Thanks for the investigation. I'm reading black's source code. It looks like black returns the original string if it contains unescaped opposite quotes:
There was a problem hiding this comment.
tbh i find python's raw string escaping rules confusing and i think there are cases that are just not properly representable (as the case above where black returns)
|
Thanks for implementing Raw-strings. Something I entirely overlooked. |
**Summary** Print the errors when the formatter ecosystem checks failed. Im not happy that we current collect the log in the first place, but this is the less invasive change and we need it to unblock reviewing #6152. **Test Plan** https://github.com/astral-sh/ruff/actions/runs/5713112075/job/15477879403?pr=6188
|
The formatter ecosystem checks are currently failing in CI. You can run the script locally ( |
|
@MichaReiser @konstin Thanks for reviewing this PR :) |
|
Fixed the end-of-string unescaped quote in #6202 |
3 participants
Summary
Fix #5941
Test Plan
Existing tests