Reading the code, when I saw the commend saying All letters used in the representation are normalized to lowercase, I thought it also refers to hex letters. However, a few lines later, I was disappointed to see Change hex literals to upper case..
There's one practical issue with use of upper-case hex letters, though, and that's similarity between letter B and digit 8.
I have seen bugs caused by this similarity in security-sensitive code, such as UTF-8/-16 decoders. Personally, I always used uppercase letters in hex numerals prior to that incident, because of the aesthetics of it; but, since then, have been switching to lowercase anywhere possible.
Maybe Black would consider switching to lowercase hex letters, as well?
I initially implemented it that way but it was changed in #530 for a reason I no longer recall. @zsol do you remember why we made that change?
See also recent https://github.com/psf/black/issues/1514, asking for lowercase hexadecimal due to the similarity of A and 4.
I initially implemented it that way but it was changed in #530 for a reason I no longer recall. @zsol do you remember why we made that change?
https://github.com/psf/black/issues/467 was a month before #530, where lowercase hex was initially decided upon. No reasoning given, but perhaps it'll help jog the memory.
I really can't recall anymore, and couldn't find a mention of this in our discussions either, but I'm sure there _was_ discussion.
ABB4AB8A
vs
abb4ab8a
isn't this enough to settle the issue?
:-)
I'll gladly fix this if everyone agrees on it? :)
I've heard no contrarian opinions so far
I agree that lowercase is better in a vacuum. I'm not as sure if this change is worth the pain of changing Black's existing style. Every time we change Black's current style, that means people who use Black to format an existing codebase need to reformat their code, which pollutes their commit histories and might break open branches. So to decide whether to make a formatting change, we need to see if the gains from the formatting change outweigh the pains of changing existing formatting. We don't have a good framework for making that tradeoff, but it's worth thinking about.
Sure, let's think about this.
Anyway, IMO one single commit to catch up with this change is not dramatic for projects adopting black
There we go - created a PR. if you do appreciate it, please label it with hacktoberfest-accepted so i can get a sticker on my cyberbike ( :stuck_out_tongue: )
I would also argue, as a user, that the change is minor, and as we are using black to not really think about formatting, it will simply happen in the background i guess. Also if you want to be explicit in your formatting rules, then you should probably freeze the version of your formatter. This seems like a minor change, and not everyone has hex numbers in their source code.
Your call :)
Just happy that I could, hopefully, help.
EDIT: I am not sure what primer is supposed to do, but the CI step fails. If you can guide me towards fixing it, then that'd be nice.
Most helpful comment
I'll gladly fix this if everyone agrees on it? :)