Fix/no char no pos squeeze

[mkatychev]

Fixes https://github.com/sharkdp/hexyl/issues/171

• Handles squeeze with --no-characters

• Handles squeeze with --no-position

• Added error handling for write! macros

• Panels field is now usize to simplify comparing against lengths

• If position is hidden, a trailing double asterisk is shown in the last line if it is part of a squeeze:

  $ cargo run -- tests/examples/hello_world_elf64 --skip=1024 -n 4096 --plain --panels 2
  
  00 00 00 00 00 00 00 00   00 00 00 00 00 00 00 00
  **
  ba 0e 00 00 00 b9 00 20   40 00 bb 01 00 00 00 b8
  04 00 00 00 cd 80 b8 01   00 00 00 cd 80 00 00 00
  00 00 00 00 00 00 00 00   00 00 00 00 00 00 00 00
  **
                                               **

[mkatychev]

@sharkdp thoughts on adding trailing double asterisks for a last line squeeze when position is turned off?

Would turn this:

$ hexyl tests/examples/hello_world_elf64 -s 1024 -n 4096 --panels 2 --plain

  00 00 00 00 00 00 00 00   00 00 00 00 00 00 00 00
  **
  ba 0e 00 00 00 b9 00 20   40 00 bb 01 00 00 00 b8
  04 00 00 00 cd 80 b8 01   00 00 00 cd 80 00 00 00
  00 00 00 00 00 00 00 00   00 00 00 00 00 00 00 00
  **

Into this:

$ hexyl tests/examples/hello_world_elf64 -s 1024 -n 4096 --panels 2 --plain

  00 00 00 00 00 00 00 00   00 00 00 00 00 00 00 00
  **
  ba 0e 00 00 00 b9 00 20   40 00 bb 01 00 00 00 b8
  04 00 00 00 cd 80 b8 01   00 00 00 cd 80 00 00 00
  00 00 00 00 00 00 00 00   00 00 00 00 00 00 00 00
  **
                                                 **

EDIT: this is now implemented

[mkatychev]

There ended up being a lot of edge cases toggling chars, position, trailing byte padding, and number of panels. I ended up adding a trait to the Assert object to help with diffing whitespace (and pretty_assertions as a dev dependency)

https://github.com/sharkdp/hexyl/blob/fed8d5c2d322a8cd95a639b99be8cf1f7b1b3011/tests/integration_tests.rs#L16-L28

so that this:

  ---- display_settings::no_chars_squeeze stdout ----
thread 'display_settings::no_chars_squeeze' panicked at 'Unexpected stdout, failed diff original var
├── original: ┌────────┬─────────────────────────┬─────────────────────────┐
│   │00000400│ 00 00 00 00 00 00 00 00 ┊ 00 00 00 00 00 00 00 00 │
│   │*       │                         ┊                         │
│   │00001000│ ba 0e 00 00 00 b9 00 20 ┊ 40 00 bb 01 00 00 00 b8 │
│   │00001010│ 04 00 00 00 cd 80 b8 01 ┊ 00 00 00 cd 80 00 00 00 │
│   │00001020│ 00 00 00 00 00 00 00 00 ┊ 00 00 00 00 00 00 00 00 │
│   │*       │                         ┊                         │
│   │00001400│                         ┊                         │
│   └────────┴─────────────────────────┴─────────────────────────┘
├── diff:
│   ---         orig
│   +++         var
│   @@ -8 +8 @@
│   -│00001400│                         ┊                         │
│   +│00001400│                         ┊                         │
└── var as str: ┌────────┬─────────────────────────┬─────────────────────────┐
    │00000400│ 00 00 00 00 00 00 00 00 ┊ 00 00 00 00 00 00 00 00 │
    │*       │                         ┊                         │
    │00001000│ ba 0e 00 00 00 b9 00 20 ┊ 40 00 bb 01 00 00 00 b8 │
    │00001010│ 04 00 00 00 cd 80 b8 01 ┊ 00 00 00 cd 80 00 00 00 │
    │00001020│ 00 00 00 00 00 00 00 00 ┊ 00 00 00 00 00 00 00 00 │
    │*       │                         ┊                         │
    │00001400│                         ┊                         │
    └────────┴─────────────────────────┴─────────────────────────┘

                                                 **

can turn into this:

More context here: https://github.com/assert-rs/assert_cmd/issues/121#issuecomment-849937376

[mkatychev]

@sharkdp Are we anticipating https://github.com/sharkdp/hexyl/pull/173 being merged first?

[sharkdp]

@sharkdp Are we anticipating #173 being merged first?

Yes, I think that would be the best strategy overall. Sorry for that :-/.

Thank you for your PR, I will take a look soon.

[sharkdp]

173 has now been merged, but please note that there are a few more open PRs that touch similar parts of the code as well (e.g. #170). Sorry :-/

[mkatychev]

Understandable, it will take me a while to get to the conflicts at the moment unfortunately. @sharkdp I'll try to revisit this next weekend.

[sharifhsn]

For what it's worth, most of the issues covered by this PR have been fixed by the larger architectural changes. There are no longer any artifacts when lines are printed, and squeezing is properly handled in all cases. I've also standardized error handling to use ? syntax bubbling up io::Result<()>. There are only a few differences in this PR:

1. A single asterisk is printed on the squeeze line when the position panel is hidden. Changing this to a double asterisk is trivial.
2. The feature-full testing framework added into the integration tests. This will not conflict with any other recently merged or soon to be merged PR.

If there's anything I missed, let me know.

[sharkdp]

A single asterisk is printed on the squeeze line when the position panel is hidden. Changing this to a double asterisk is trivial.

Okay. I'm fine with either version. So let's keep what we have right now, unless this double asterisk has some precedent in other apps. Or if there is any other advantage.

The feature-full testing framework added into the integration tests. This will not conflict with any other recently merged or soon to be merged PR.

@mkatychev Do you think you could extract those changes into a new PR?

@sharifhsn Or are these things already covered in your new unit tests for squeezing?

[sharifhsn]

There are some cases not covered by my tests, like squeeze with incomplete last line. And in general, more testing is a good thing, especially since all my tests were unit, not integration. The only changes would have to be the asterisks when printing without position panel.

[mkatychev]

@sharkdp I'll start with the tests in a new branch and see what breaks then.