Repository navigation
Suggestion: use lowercase letters in hex literals #1692
Description
Activity
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
abb4ab8aisn't this enough to settle the issue?
:-)I'll gladly fix this if everyone agrees on it? :)
Reacted by Iago-lito, Behnam Esfahbod, Ferdinando Ametrano and Jimmy ThrasibuleReacted by Jimmy ThrasibuleI'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 blackReacted by Iago-lito, Jimmy Thrasibule and Ajinkya KhocheThere we go - created a PR. if you do appreciate it, please label it with
hacktoberfest-acceptedso i can get a sticker on my cyberbike ( 😛 )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
primeris supposed to do, but the CI step fails. If you can guide me towards fixing it, then that'd be nice.Reacted by Iago-litoReacted by Iago-lito and Behnam Esfahbod- added a commit that references this issue
on Apr 25, 2021 I was the original advocate for upper-case numerals and based on my whining we decided to revert this change. The responsibility for the disruption is mine alone, I'm sorry to have wasted your time.
Line of reasoning:
- numeral formatting introduced in 2018 has been used with close to zero feedback that it's wrong;
- there's hundreds of millions of lines of code (in a very literal sense) formatted with Black now, many of which depend on the current formatting;
- apart from aesthetic arguments, the readability argument is somewhat dubious, if your programming font lets people mistake 4 for A, 8 for B, or 0 for C, you should be changing it as there's a much wider area of confusion than hex numerals;
- the "security issues" argument in the docstring is not based on any evidence AFAICT.
Reacted by Steven Marshall and NiclasHadererReacted by Steven Marshall and Erasmus CedernaesMaybe we can revisit this before we go "stable" one last time and if (and only if) we get a lot of the community behind this (maybe can do a survey or something) we could do a dedicated release for this case change.
- added a commit that references this issue
on Jun 6, 2021 - added a commit that references this issue
on Apr 17, 2026
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 seeChange hex literals to upper case..black/src/black/__init__.py
Lines 5158 to 5168 in 6284953
There's one practical issue with use of upper-case hex letters, though, and that's similarity between letter
Band digit8.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?