search

sharkdp / hexyl

A command-line hex viewer
Apache License 2.0
8.92k stars 227 forks source link

[feature] Implement alternate bases #178

Closed sharifhsn closed 1 year ago

sharifhsn commented 1 year ago

Fixes #147.

This PR provides the new --base/-b argument which specifies the base that the bytes should be printed in. The base options exposed by the standard library are binary, octal, decimal, and hexadecimal. If we were to add more options, then we would need to pull in some kind of dependency.

For ease of use, the argument accepts either a number (2, 8, 10, 16), a single character abbreviation (b, o, d, x), a three-character abbreviation (bin, oct, dec, hex), or the full base name (binary, octal, decimal, hexadecimal).

Part of this PR is adjusting the --panels=auto option to account for both base and byte grouping from #170. Now it is guaranteed to give the correct number of panels regardless of options.

I have also added a simple integration test.

Questions for review:

This will be my last PR until the next release of hexyl.

sharkdp commented 1 year ago

Should more bases be allowed?

I think the supported set is completely reasonable. I don't expect anyone to require base 5 or similar.

What should the help text suggest for the argument format?

You're talking about the value_name? I think B is fine. Maybe for the values, we could support either the name (hexadecimal) or a number (16), but that's not really important for now. I'm completely fine with your proposal.

It seems pretty obvious, but maybe we could mention the default (hexadecimal).

Should there be more tests?

I think we're good for now - thank you!