search

k0001 / pipes-binary

Encode and decode binary streams using the pipes and binary libraries.
Other
7 stars 12 forks source link

Lens-based encoding/decoding #10

Closed jdnavarro closed 10 years ago

jdnavarro commented 10 years ago

I'm opening this so that you are aware, I'm trying some improvements based on the new lens-based parsing in pipes-bytestring.

https://github.com/jdnavarro/pipes-binary/commits/master

Right now the tests are broken, but will send a pull request when/if I get things right.

k0001 commented 10 years ago

Thank you Danny. I'll take a look and catch up with your changes.

k0001 commented 10 years ago

@jdnavarro I've updated the k0001/master branch of pipes-binary including some of your changes. I think pipes-binary is now ready to be pushed to Hackage.

Is it OK if I add your name to the PEOPLE file?

jdnavarro commented 10 years ago

Glad some changes were useful. You can add me to PEOPLE, thanks.

PierreR commented 10 years ago

Hi,

I am not sure where is the best place to discuss this (maybe the mailing list ?) but I am wondering about the possibility to get the function isEndOfBytes' in pipes-bytestring as it is duplicated in pipes-binary, pipes-attoparsec and pipes-aeson.

isEndOfBytes':: Monad m => S.StateT (Producer ByteString m r) m (Maybe r)

As a personal note, I have found the name and pattern a bit confusing. Because is usually implies a bool and here Nothing means "there is more input".

What do you think ?

Cheers,

k0001 commented 10 years ago

@PierreR isEndOfBytes' is just an optimization, and I agree that it is confusing, which is why I've kept it as an internal function. Maybe I could rename it to atEndOfBytes internally. Anyway, I think I wouldn't like that function in pipes-bytestring even if it had a different name, mostly because it doesn't fit in the Parser synonym, which is confusing.