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

ByteString.Char8 encoder #74

Open
hasufell opened this issue Jan 12, 2020 · 6 comments
Open

ByteString.Char8 encoder #74

hasufell opened this issue Jan 12, 2020 · 6 comments
Labels
enhancement New feature or request

Comments

@hasufell
Copy link

I'm surprised there exists no encoder for that: https://hackage.haskell.org/package/waargonaut-0.8.0.1/docs/Waargonaut-Encode.html#g:4

@mankyKitty
Copy link
Contributor

I'm not sure that's really "issue" worthy, but I would happily accept a PR for such a thing. 👍

import qualified Data.ByteString.Char8 as B8

encodeByteStringChar8 :: Applicative f => Encoder f B8.ByteString
encodeByteStringChar8 = B8.unpack >$< string

:)

@hasufell
Copy link
Author

Well, this converts to string. I was wondering if there could be a more efficient low-level encoder.

@mankyKitty
Copy link
Contributor

Quite possible, as ByteString.Char8 is limited to the latin1 charset which is a subset of what the JSON RFC allows. So you could see how well it goes to just drop it directly into the encoded structure.

Have you encountered any efficiency issues with Waargonaut?

@hasufell
Copy link
Author

Have you encountered any efficiency issues with Waargonaut?

Not really, it's my first time trying to use it.

So you could see how well it goes to just drop it directly into the encoded structure.

Yeah, but the internal API is a bit complicated ;)

@mankyKitty
Copy link
Contributor

Not really, it's my first time trying to use it.

Phew and yay!

Yeah, but the internal API is a bit complicated ;)

Yeah... it is a bit.. ahem :<

Part of this is due to a slightly crazy adherence to the RFC for JSON. Hence the wacky level of detail in the string types etc. As well as being able to support round trip parsing & printing, which meant trying to find a way to remember where all the whitespace is, and what type of whitespace it is.

@mankyKitty
Copy link
Contributor

One option could be to change the type alias here https://github.com/qfpl/waargonaut/blob/master/src/Waargonaut/Types/JString.hs#L78 to be a sum type that is either the internal JString' representation or a narrower NonUnicode type... maybe:

data JString
  = JString' HeXDigit
  | NonUnicode Char8.ByteString

@emilypi emilypi added the enhancement New feature or request label Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants