sharkdp / hexyl

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

Add more formatting options #149

Open Aaron-Rumpler opened 2 years ago

Aaron-Rumpler commented 2 years ago

Adds the following options:

TODO:

sharkdp commented 2 years ago

Thank you for your contribution. Could you explain the motivation for this change in a bit more detail? And maybe show a few screenshots how the modified output looks like? Or is there an open ticket that discusses this?

Aaron-Rumpler commented 2 years ago

The primary motivation is that I just find the inner separator on the text column kind of annoying sometimes. I also wanted to be able to output without the top and bottom borders (so that line 1 of the output would correspond to the first line of the actual hex dump), but still keep the separator lines.

Aaron-Rumpler commented 2 years ago

hexyl hex text hex_text outer_border hex_text_outer_border

Aaron-Rumpler commented 2 years ago

Only just realised, but combining --hex-inner-separator none, --text-inner-separator none and --outer-border none results in an output very similar to hexdump -C: hexdump

sharkdp commented 2 years ago

Thank you very much for the explanation. I'm inclined to accept a change like this, but I would like if we could think about the command-line interface a bit. For example: is there any way we could avoid introducing multiple new options for this?

I also have some questions when looking at the screenshots:

hex_text

should --hex-inner-separator none also get rid of the longer whitespace separator between the left and right hex block? to be consistent with --text-inner-separator none?

outer_border

shouldn't this remove the right border as well?

Aaron-Rumpler commented 2 years ago

I would like if we could think about the command-line interface a bit. For example: is there any way we could avoid introducing multiple new options for this?

I agree wrt the command line interface, which is the primary reason this is still a draft PR.

hex_text

should --hex-inner-separator none also get rid of the longer whitespace separator between the left and right hex block? to be consistent with --text-inner-separator none?

The argument against this would be hexdump -C, which also has the double space gap between the left and right hex block, but no gap on the text.

hexdump

outer_border

shouldn't this remove the right border as well?

With the current spacing, I found doing that to look quite unbalanced.

hex_text_outer_border_no_right_pipe

We could do something like this:

hex_text_outer_border_no_right_pipe_new_spacing

There is an issue with not having a right border though (which also affects --border none). With input like this it's hard to tell from the text alone that there's a space there.

hex_text_outer_border_no_right_pipe_issue

Again, hexdump -C has a left and right border on the text, even though it has no other borders.

hexdump_trailing_space_line

sharkdp commented 2 years ago

should --hex-inner-separator none also get rid of the longer whitespace separator between the left and right hex block? to be consistent with --text-inner-separator none?

The argument against this would be hexdump -C, which also has the double space gap between the left and right hex block, but no gap on the text.

okay :+1:

shouldn't this remove the right border as well?

With the current spacing, I found doing that to look quite unbalanced.

Okay, so maybe we rather use something like --outer-border=none/left/right/both and let the user decide?

There is an issue with not having a right border though (which also affects --border none). With input like this it's hard to tell from the text alone that there's a space there.

Good point. Any suggestions?

Regarding the CLI: can we write down some alternative proposals and comare them? Feel free to also suggest breaking changes (like a reuse of the existing --border option).

Aaron-Rumpler commented 2 years ago

There is an issue with not having a right border though (which also affects --border none). With input like this it's hard to tell from the text alone that there's a space there.

Good point. Any suggestions?

We could print a trailing symbol (like ␄, though there are probably much better options) after the end of the file. There is the case where it ends up on a new line, in which case you just wouldn't show it, as the input ends at the end of a line.

hexdump -C does print a trailing line that includes the total input size, which might be a good idea to offer. In that case, you'd print the ␄ regardless.

hexdump_trailing_space_line

Though talking about the ␄ character, there could be an option to display those for control characters (it's not like you could confuse those for that character appearing in the input itself, as it's Unicode).

Aaron-Rumpler commented 2 years ago

shouldn't this remove the right border as well?

With the current spacing, I found doing that to look quite unbalanced.

Okay, so maybe we rather use something like --outer-border=none/left/right/both and let the user decide?

Regarding the CLI: can we write down some alternative proposals and comare them? Feel free to also suggest breaking changes (like a reuse of the existing --border option).

I'm partial to replacing --border with something more like what bat does for --style. Could have left, right, top, bottom, hex-inner, text-inner. none would simply disable all the borders, and there'd be options for enabling and disabling groups as well. I think the ASCII/Unicode option should be called something different, as it affects more than just the borders.