Skip to content

std.fmt.hex: support integer sizes that are not powers of two #23819

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

myclevorname
Copy link

Fixes #23799

This changes the return type, but I don't know of any targets where this is actually breaking change.

@RetroDev256
Copy link
Contributor

Would we also want to support integer sizes that are not a multiple of 8? Not exactly saying we should, but I am wondering if there is merit to making this work for all unsigned integers.

@myclevorname
Copy link
Author

Would we also want to support integer sizes that are not a multiple of 8? Not exactly saying we should, but I am wondering if there is merit to making this work for all unsigned integers.

That's a good idea. Let me work on that.

@myclevorname myclevorname marked this pull request as draft May 6, 2025 22:15
@wooster0
Copy link
Contributor

wooster0 commented May 7, 2025

While you're at it do you mind adding a case: Case parameter for allowing uppercase too? It's not difficult to support.

@myclevorname
Copy link
Author

While you're at it do you mind adding a case: Case parameter for allowing uppercase too? It's not difficult to support.

My idea was to introduce a hexOptions function with options for that, and then reimplementing hex on that. I am balancing school, life, and hobbies, so this won't get done very quickly.

@wooster0
Copy link
Contributor

wooster0 commented May 7, 2025

Not sure which option this function would ever need beside case. Also I'm guessing that would be biased towards lowercase as the default which I can't say I'm a fan of. It's fine to break code if you're worried about that.

@myclevorname myclevorname marked this pull request as ready for review May 7, 2025 20:08
@myclevorname
Copy link
Author

Not sure which option this function would ever need beside case. Also I'm guessing that would be biased towards lowercase as the default which I can't say I'm a fan of. It's fine to break code if you're worried about that.

By default, hexOptions is lowercase and big-endian. hex uses little-endian and lowercase as before, and is deprecated by hexOptions(x, .{ .endianness = .little }).

@myclevorname myclevorname force-pushed the hex-fmt branch 3 times, most recently from e9b0a43 to 6acc69d Compare May 8, 2025 19:15
@myclevorname
Copy link
Author

Marking draft until I fix all the bugs :/

@myclevorname myclevorname marked this pull request as draft May 9, 2025 09:54
@myclevorname myclevorname marked this pull request as ready for review May 10, 2025 13:57
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 this pull request may close these issues.

std.fmt.hex does not work for integers that are not powers of two
3 participants