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

Discrepancy in upper/lower case in impl fmt::Debug for BStr #188

Open
wbenny opened this issue Jul 25, 2024 · 1 comment · May be fixed by #189
Open

Discrepancy in upper/lower case in impl fmt::Debug for BStr #188

wbenny opened this issue Jul 25, 2024 · 1 comment · May be fixed by #189

Comments

@wbenny
Copy link
Contributor

wbenny commented Jul 25, 2024

Hi,

I found a discrepancy in upper/lower case within debug formatting of BStr:

https://github.com/BurntSushi/bstr/blob/master/src/impls.rs#L483-L493

While this is not that big of a deal (we can use to_lowercase/uppercase), it can be an annoyance :).

Would it be possible to unify the case-ness of the output? If so, what case should be preferred? It would be great if this caseness could be controlled - maybe via {:X?} and {:x?}? Although I'm not sure whether it's even possible.

Would it be a breaking change?

@BurntSushi
Copy link
Owner

I think we'd probably want to implement the UpperHex (and related) traits. Then I believe you could use {:X?} with a &BStr. I think that's probably fine. It's not quite a perfect fit if you read the docs of UpperHex though, since it seems to imply that it's for formatting numbers and this is for strings.

Aside from that though, yes, absolutely, we can unify the case. That should be fixed regardless. It was probably just an oversight. I haven't noticed it because it looks like the uppercase variant is very rare. So to that end, I'd be happy to accept a PR that switches everything to lowercase.

wbenny added a commit to wbenny/bstr that referenced this issue Jul 28, 2024
Make all of the debug output in lower-case.

Closes BurntSushi#188
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 a pull request may close this issue.

2 participants