Suggestion: use lowercase letters in hex literals

[besfahbod]

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..

black/src/black/__init__.py

Lines 5158 to 5168 in 6284953

	All letters used in the representation are normalized to lowercase (except
	in Python 2 long literals).
	"""
	text = leaf.value.lower()
	if text.startswith(("0o", "0b")):
	# Leave octal and binary literals alone.
	pass
	elif text.startswith("0x"):
	# Change hex literals to upper case.
	before, after = text[:2], text[2:]
	text = f"{before}{after.upper()}"

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?

[JelleZijlstra]

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?

[hugovk]

See also recent #1514, asking for lowercase hexadecimal due to the similarity of A and 4.

[hugovk]

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?

#467 was a month before #530, where lowercase hex was initially decided upon. No reasoning given, but perhaps it'll help jog the memory.

[zsol]

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.

[fametrano]

ABB4AB8A
vs
abb4ab8a

isn't this enough to settle the issue?
:-)

[C0DK]

I'll gladly fix this if everyone agrees on it? :)

[fametrano]

I've heard no contrarian opinions so far

[JelleZijlstra]

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.

[fametrano]

Sure, let's think about this.
Anyway, IMO one single commit to catch up with this change is not dramatic for projects adopting black

[C0DK]

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 ( 😛 )

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.

[ambv]

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.

[cooperlees]

Maybe 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.

[C0DK]

Man, I hoped I would be a dedicated contributor. but that's all fine :) I do see your point @ambv and agree.