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

Feedback on lowercase \U, \u, \x escapes introduced in 23.1 preview style #2916 #3568

Closed
yilei opened this issue Feb 13, 2023 · 4 comments
Closed
Labels
C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. T: style What do we want Blackened code to look like?

Comments

@yilei
Copy link
Contributor

yilei commented Feb 13, 2023

I'm not sure if lowercase is the better decision, so I'm filing this feedback for discussion.

Reasons to favor lowercase:

  1. repr() uses lowercases:

    >>> "\x1B"
    '\x1b'
    >>> "\u200B"
    '\u200b'
    >>> "\U0001F977"
    '\U0001f977'
  2. I sampled 500K files from our codebase, lowercase is bit more popular (the count below is the total number escape sequences in strings):

    \x escapes:

    • lowercase: 440K
    • uppercase: 351K
    • no letters: 341K

    \u escapes:

    • lowercase: 144K
    • uppercase: 49K
    • no letters: 46K

    (This might actually be a reason to not have a preference, since this is polarized.)

Reasons to favor uppercase:

  1. For \x escapes, Black already uses uppercase for hex numbers: 0xEF, so this will be consistent with Black itself.
  2. For unicodes, https://unicode.org/ uses uppercases.

I personally think the uppercase reasons are stronger.

@yilei yilei added the T: style What do we want Blackened code to look like? label Feb 13, 2023
@JelleZijlstra JelleZijlstra added the C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. label Feb 13, 2023
@JelleZijlstra
Copy link
Collaborator

For reference #2916 and #2067 is where this was added. I don't have a strong opinion here but this issue will provide a good way to gather user feedback while this is in the preview style.

@JelleZijlstra
Copy link
Collaborator

Thinking about this more and re-reading the original issue, I agree that we should use lowercase for these. I propose that we keep this change out of the 2024 stable style and make a preview change to lowercase.

@JelleZijlstra
Copy link
Collaborator

I think I misread something here, we already use lowercase in preview style, though @yilei above weakly prefers uppercase. I think the reasons for lowercase are a little stronger, and this has been in preview for almost the whole year with no complaints, so I'm inclined to stick with lowercase.

@yilei
Copy link
Contributor Author

yilei commented Dec 10, 2023

Feel free to close, as we haven't heard other feedback about this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

2 participants