Skip to content
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

prevent SyntaxWarning on character escapes #94

Merged
merged 4 commits into from
Mar 22, 2025

Conversation

afeistel
Copy link
Contributor

@afeistel afeistel commented Mar 22, 2025

most fixes involve r- (raw) strings, but one had to use \\ because it also contained a proper escape, \n, and doing r"..." "\n" felt like too much of a hack

r"..." is picked over \\ for consistency with the convention of using r"..." with re

@afeistel
Copy link
Contributor Author

(I went through this before #93, trying to work on #92)

@HuyaneMatsu
Copy link
Owner

Raw strings are a trap if you think about it.

Example: rf / fr string prefix makes no sense. You can see here that even tho escapes are ignored, { is still preserved for f strings, and requires to you use { to escape {. The point of raw strings would be that you should not be required to do this. 🤦🏻

In [0]: print(r'hello\n\n\t\t\\')
hello\n\n\t\t\\
In [1]: print(rf'hello\n\n\t\t\\{}')
  File "<console in[1]>", line 1
    print(rf'hello\n\n\t\t\\{}')
          ^
SyntaxError: f-string: empty expression not allowed
In [2]: print(rf'hello\n\n\t\t\\{{}}')
hello\n\n\t\t\\{}

Please use \\ instead :)

@afeistel
Copy link
Contributor Author

changed non-re cases to \\

@HuyaneMatsu
Copy link
Owner

I do not wanna merge the PR and then remove the r strings after. I think it would not make you feel good.
But you already put in the work and me just fixing the same stuff would also not make you feel good.

I just really do not wanna see any r strings.

@HuyaneMatsu HuyaneMatsu merged commit 68a2291 into HuyaneMatsu:master Mar 22, 2025
@HuyaneMatsu
Copy link
Owner

Thanks for your contribution. And I am sorry because the administration took way longer than the actual fix.

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.

2 participants