Error handling

[tcharding]

Is it ok that HexToBytesError does not use non_exhaustive. I think it is but could I get a few more eyes on it please.

[apoelstra]

Yeah, I think this is reasonable. There are only so many ways that hex decoding could go wrong and these variants all contain plausibly useful information for users to match on.

[tcharding]

If #31 merges I believe we can close this.

[Kixunil]

Yeah, it's good, I just want to wrap each variant in a struct. Especially Char one where I want to support reporting multiple chars eventually (with feature flags and such).

[tcharding]

I just want to wrap each variant in a struct.

The statement below assumes by this you mean "hide the internals", same as the reasoning for adding non_exhaustive.

So I thought about the "non_exhaustive on all errors so that we can add information" thing and I thought to myself, "given the amount of time its taken to get the errors to this state it might be reasonable to think that the extra information stuff will not happen for a while, especially considering it can be done org wide. Why not just release it as a new major version?" - what do you think?

[Kixunil]

If the intention is to release this ASAP as 0.x, then I'm fine. But for 1.0 I want the internals hidden. I specifically know what I want to add eventually (spans), so I see no reason to not protect the stability.

However, I'm considering allowing in the future to be able to report both uneven length and wrong characters, which would imply hiding whole error and then having a method to get the first error (which is always available) and a method to iterate over the errors when multi-span feature is enabled.

[apoelstra]

Yes, for 1.0 we definite want to hide the errors properly and add as much extra information as we can.

@tcharding I wouldn't mind doing some small things in order to get a quick release out, but I wonder if we actually need a quick release here? This crate is functionally complete and the remaining TODOs are API improvements for 1.0, so maybe we should just grind away on that and not bother releasing til we think we're close to done?

[tcharding]

Oh I wasn't pushing for another 0.x release, was just referring to the eventual 1.0 release - which I am pushing for :)

[tcharding]

Oh boy, I was just working on this and I realized that even wrapping the errors in structs does not save us from having to do a major version bump when we add information because doing so almost certainly changes the Display implementation which is part of the public API (I read this somewhere recently).

[Kixunil]

@tcharding I'm not convinced Display implementation has to be part of public API, especially for errors but the solution is simply to not put that information into Display and provide a different way to access it.

For spans I would absolutely not put them into Display anyway (unless they were "on line x, character y" but we won't have those).

[tcharding]

but the solution is simply to not put that information into Display and provide a different way to access it.

Oh, that is a good point. Did not think of that. Cheers.

[tcharding]

Closing in favour of #42