Align fmt_hex_exact and Display impls

[tankyleo]

fmt_hex_exact accounts for precision.

DisplayArray and DisplayByteSlice add the "0x" prefix if the '#' (alternate) flag is set.

Close #83

[tankyleo]

• I set it in draft for now - this is where I am currently, but I understand there may be concerns with this direction. I am not thrilled about this PR myself.
• I keep the core display engines of fmt_hex_exact and DisplayArray / DisplayByteSlice separate: fmt_hex_exact takes advantage of operating on a known length array, and DisplayArray / DisplayByteSlice take advantage of operating on a &[u8].
• write_pad_left and write_pad_right implement the common padding functionality in both cases.

[tcharding]

This looks like its going the correct direction mate. Currently its not obvious to me if this changes the logic. Perpaps split it up into separate commits

1. Do the is_full changing
2. Add some regression tests that show the current behaviour if needed
3. Refactor without changing the behaviour
4. Change the logic to fix the issue

As far as the refactor goes something like this would probably be nice

fn internal_display() {
    use fmt::Write as _;

    let mut encoder = BufEncoder::<1024>::new(case);
    let pad_right = write_pad_left(f, bytes.len(), &mut encoder)?;
    write_data(/* ... */)?;
    if pad_right > 0 {
        write_pad_right(/* ... */)?;
    }
    Ok(())
}

Also, can you layout the functions in the order they are called i.e put internal_display (or display_slice if you like that name) first then the other helpers below it. This is based on the idea that the Rust compiler doesn't care so its easier on the reader to put the code in the same order that they read it. Or said another way, the important stuff first. Thanks man

[tcharding]

This looks wrong, you write the prefix after having called write_pad_left. If I am correct then it hints that this PR needs more tests because tests are currently passing.

[tankyleo]

Hmm are you sure ? Seems to me we want to 1) write left padding spaces 2) write the 0x 3) write the hex string 4) write the right padding spaces.

[tcharding]

derp - I'm a wombat. My bad.

[tankyleo]

No problem appreciate your review.

[tcharding]

Unrelated change, should be in a separate commit.

[tcharding]

I rekon these comment should be written up into an issue (or 2) and removed - assuming you understand them. (I would have to spend some time thinking to work them out.)

[tankyleo]

In the commits below I keep these comments around - if that's ok with you we can address these in a follow-up PR.

[tcharding]

Sure thing.

[tankyleo]

Hello, here I've split the PR into likely too many commits - happy to squash later, but I hope this helps clarify what I'm doing.

    if pad_right > 0 {
        write_pad_right(/* ... */)?;
    }

I prefer to keep that if statement inside the write_pad_right function to guard against division by 0. Otherwise, we could do as shown above, and add an assert statement inside the function ?

write_data(/* ... */)?;

Here I prefer to avoid creating a new function if that function only gets called in a single place throughout the codebase.

[tcharding]

Man this is awesome, super easy to review now. Well done. Since you've made it this far how about throwing a unit test after each of the last 2 patches that test the changes. We put the unit test after the patch that does the fix for 2 reasons:

1. So that after merge all patches on master still build and pass tests
2. So that reviewers can re-order the patches and verify that the unit test fails without the fix.

[apoelstra]

ACK 228a40c; successfully ran local tests

[tcharding]

ACK 228a40c

[tcharding]

This is a nice patch set, top effort. You even made all the test code uniform, my man!

[tankyleo]

I thank you for your encouragement and guidance.

Happy to help push this library to 1.0.