Add InvalidCharError::char

[tankyleo]

Returns a char if the invalid byte is ascii, otherwise returns the raw u8.

Deprecates InvalidCharError::invalid_char.

Partially addresses #100

[tankyleo]

This does not handle multi-byte characters yet.

• All HexToBytesIter needs is a T: Iterator<Item = [u8; 2]> + ExactSizeIterator, as shown in HexToBytesIter::from_pairs. I am reluctant to add more constraints on the T to peek for multi-byte characters in an error scenario.
• Even if we did handle multi-byte characters, not all of them would result in a nice visual picture. Is it worth the trouble then to handle multi-byte chars? I am not convinced.

[tankyleo]

Is HexToBytesIter::from_pairs meant to be part of the public API ? Right now it is a pub fn inside a private module.

[apoelstra]

This does not handle multi-byte characters yet.

In this case the returned char is incorrect and we can't move forward with this approach.

[tankyleo]

In this case the returned char is incorrect and we can't move forward with this approach.

At the moment we only return a char if it is a single-byte character right ?

I agree that multi-byte chars would need more work.

[apoelstra]

At the moment we only return a char if it is a single-byte character right ?

No, we currently do not return a char under any circumstances, because the error type does not contain enough information to return a char.

[tankyleo]

Right I was talking about this current commit here all good.

[apoelstra]

Oh, I see! I misunderstood your initial approach.

This is clever but I am still going to concept-NACK it. If users want to do this (treat ASCII specially) it is easy for them to do it themselves -- but this change forces them to do so, which adds extra code complexity.

It's also an ugly API. While the motivation makes sense, in terms of our existing code layout which prevents us from unconditionally getting a char, I don't think this is something we want to expose in a 1.0 version of the crate.

[tankyleo]

Sounds great I agree.

Was there another approach you had in mind for #100 ? If I understand you correctly we would not address this issue.

[apoelstra]

What I would imagine is that if we read an invalid u8, we attempt to continue reading u8s until we've parsed a complete UTF-8 codepoint. (We would not attempt to deal with combining characters or anything like that because Rust itself makes it hard.)

And if we fail that...hmm. Maybe we would want to fall back to your API anyway? Or I think there's a standard UTF "unknown character" character we could just sub in.

[tcharding]

@apoelstra we chatted about this, bump so you can update your position if you want to.

[apoelstra]

Yeah -- I think we don't want to target this issue at all now for 1.0. If there is some noninvasive non-performance-affecting way to do it, that's great, but I don't believe there is.

Meanwhile, if a user has a hex string with a bad character in it, and wants to learn more specifically what that character is (and where it is, etc), it's not hard to do this without help from the crate. (s.chars_indices().find(|(_, x)| !x.is_ascii()) will do it for example).

[tankyleo]

Sounds good, I close this PR.

Whenever appropriate, I let you close #100 as well.