Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add some utf8 decoding convenience functions #342

Merged
merged 1 commit into from
May 25, 2021

Conversation

pearcedavis
Copy link
Contributor

Addresses #341.

Let me know if I should add any INLINE pragmas or "since" notes in the docs.

Lysxia
Lysxia previously approved these changes May 25, 2021
@Lysxia
Copy link
Contributor

Lysxia commented May 25, 2021

Looks good to me. I assume @haskell/text won't object to this.

@pdedmon Can you resolve the conflicts?

@Lysxia Lysxia added this to the 1.3.0.0 milestone May 25, 2021
@Bodigrim
Copy link
Contributor

Bodigrim commented May 25, 2021

decodeUtf8Lenient is fine, but I'm really unsure how much our vocabulary benefits from a synonym and a trivial composition. I do not object however.

@Lysxia
Copy link
Contributor

Lysxia commented May 25, 2021

It's arguable that decodeUtf8' is not a good name (another issue complaining about it), whereas if you see decodeUtf8Either you can better guess what it does. I do share your reservations about also having a Maybe version though...

@pearcedavis
Copy link
Contributor Author

Shall I reduce this down to just decodeUtf8Lenient then? I don't feel too strongly about the others, though I do agree with the sentiment around decodeUtf8'.

@Lysxia
Copy link
Contributor

Lysxia commented May 25, 2021

Let's do that, yes.

@pearcedavis
Copy link
Contributor Author

Done and rebased.

@Lysxia Lysxia merged commit d7bcf44 into haskell:master May 25, 2021
@pearcedavis pearcedavis deleted the additional-decoding-helpers branch May 25, 2021 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants