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 safe decodeASCII' #499

Merged
merged 1 commit into from
Feb 9, 2023
Merged

add safe decodeASCII' #499

merged 1 commit into from
Feb 9, 2023

Conversation

raehik
Copy link
Contributor

@raehik raehik commented Jan 28, 2023

Useful function for serialization libs. Currently you have to do something like this, which is nasty and weird for something that isn't really inherently so.

I find the decoding setup to be a little messy and this makes no attempt to improve on that. I wouldn't mind assisting on that work too.

@raehik
Copy link
Contributor Author

raehik commented Jan 28, 2023

decodeAsciiPrefix in #448 would also resolve this, I doubt anyone minds about a little extra allocation for the fail case.

@Lysxia
Copy link
Contributor

Lysxia commented Feb 4, 2023

And the extra allocation in #448 would get optimized away if you just check the suffix for emptiness.

@raehik
Copy link
Contributor Author

raehik commented Feb 7, 2023

With #448 merged, I'll rebase this and see what's left -- perhaps nothing, or perhaps a convenience definition if it seems sensible.

@Lysxia
Copy link
Contributor

Lysxia commented Feb 7, 2023

Yeah, I think a Maybe or Either variant of decodeASCII makes sense even if it's a simple wrapper around decodeASCIIPrefix, because it has a more natural-looking type.

@raehik raehik force-pushed the decodeascii-safe branch 2 times, most recently from c09ef52 to 1e12669 Compare February 8, 2023 14:41
@raehik
Copy link
Contributor Author

raehik commented Feb 8, 2023

Thanks to @Lysxia 's fantastic work on decoding there's not much here now:

decodeASCII' :: ByteString -> Maybe Text
decodeASCII' bs =
  let (prefix, suffix) = decodeASCIIPrefix bs in
  if B.empty suffix then Just t else Nothing
{-# INLINE decodeASCII' #-}

What do people think? It's perhaps more convenient, but gives less information than decodeASCIIPrefix.

src/Data/Text/Encoding.hs Outdated Show resolved Hide resolved
@raehik raehik marked this pull request as ready for review February 9, 2023 12:28
@raehik
Copy link
Contributor Author

raehik commented Feb 9, 2023

pushed fix B.empty -> B.null lol and re-squashed

@Bodigrim Bodigrim merged commit 28b5088 into haskell:master Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants