Add structured types for bech32 encoding result

[tochicool]

Attempts to resolve #39

This introduces the bech32EncodeResult and bech32DecodeResult functions and their respective structured types to allow for custom application handling of failures. This also exports the maxBech32Length definition.

[GambolingPangolin]

I really like the new functions. I'm going to be looking out for other places where we can succeed with errors instead of failing outright :)

[GambolingPangolin]

I am on the fence about explicitly ambiguous record fields. The case for allowing it here is pretty strong:

• The *Result types are unlikely to be used as type-class instances
• They are likely to be consumed quite near where they are produced

Some inconveniences:

• In order to use, e.g. result for both types in the same module DisambiguateRecordFields is needed (correct me here if I'm wrong)
• Downstream code might require more explicit type annotations as a result

[tochicool]

Yup, DisambiguateRecordFields can be used if you want to use the record field but have both record fields imported in your module with the same, or no, qualifier. Since GHC 9.20, you can also now also just turn on OverloadedRecordDot and use x.result anywhere without any real downsides. I tend to turn on DuplicateRecordFields very liberally, but it would be nice if record fields had their own namespace and weren't automatically functions with the same without some explicit intent.

How do ambiguous record fields interact with type class instances that make them more inconvenient? Just curious.

[GambolingPangolin]

Yeah it would be great if you could disambiguate by using Ctor.someField or something.

Hm. I guess it's just an intuitive concern that ambiguous record fields can hurt type inference. Sometimes code is accidentally more general than it seems. Suppose you have a type class where Bech32EncodeResult is a instance but Bech32DecodeResult is not. If you wrote some code (unannotated like in a where clause) which was accidentally polymorphic over these two types, it would not be possible to compose it with the type class members without providing an additional annotation. To contrast, if the fields are unambiguous the code might end up being monotype instead.

[tochicool]

I see. I've pushed the changes with the field names disambiguated. Added it for all fields to keep it consistent as they may share more fields later on.

[GambolingPangolin]

Well done

[tochicool]

Please squash the commits!

[ProofOfKeags]

Do we wanna make this !Text?

[GambolingPangolin]

I think there is a good case to be made for making this field lazy. After all consumers may check one of the error fields and decide to throw away the encoding. Maybe we can reduce indirection for bech32Encode by changing the last line to

encodeResult `seq` pure encodeResult

[tochicool]

Is the reason for wanting seq there to reduce some unwanted laziness? I think the following would no longer terminate if encodeResult was somehow not total, where it would have previously otherwise, ie:

case bech32Encode input of { Just _ -> "Valid bech32"'; Nothing -> "Invalid bech32" }

Happy to go with that if we're all convinced that encodeResult is always total. I'm also fine with keeping existing definitions for bech32Encode / bech32Decode if we're worried about the extra overhead caused by the lazy data types.

[GambolingPangolin]

That was a misguided suggestion. Let's leave things as is. FWIW that making encodeResult a strict field has the same effect with respect to making bech32Encode diverge.

[ProofOfKeags]

My suggestion was rooted in the perhaps false assumption that we could predict that the overwhelming majority of the time we would want to read the decoded result out. If that isn't the case then we can leave it as is.