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

Data.Attoparsec.ByteString.Streaming.parse uses left-biased Either #1

Open
torgeirsh opened this issue Nov 19, 2016 · 9 comments
Open

Comments

@torgeirsh
Copy link

Is there a reason, or just an oversight?

@michaelt
Copy link
Owner

michaelt commented Nov 19, 2016

It's an oversight. Here and in a couple other modules here I am refactoring pipes material, where generally Either is used as an unordered sum type - not envisaging a use of the monad instance or with any suggestion that material on the left is error-like. The results that are sometimes counter-intuitive. So I was bewitched into an indifference which is here pointlessly confusing: of course with a parser the result should be Either Message a! I'm not sure what the best path to repair this is. I am slowly accumulating material for a large version bump for all the streaming libraries but was thinking of delaying a few months.

@torgeirsh
Copy link
Author

Since swapping the Either is a breaking change, I'm sure it can wait until the large bump. Another thing that would be good to have for the large bump is parsing with length, i.e. parseL and parsedL, similar to pipes-attoparsec.

By the way, how do you feel about splitting up the package? It currently brings in a lot of unrelated functionality. I wanted to stream an attoparsec parser, but was unable to compile streaming-utils on Windows because one of the JSON dependencies was POSIX only. The JSON dependency is fixed now, but it's still something that isn't needed for attoparsec support.

@fosskers
Copy link

fosskers commented Mar 6, 2017

Came here to open this issue, to find it already open! Just echoing that left-biased Either is weird.

@michaelt
Copy link
Owner

michaelt commented Mar 6, 2017

Right sorry, I got bogged down in an excess of choice points. My decision was just to make a separate streaming-attoparsec package. https://github.com/michaelt/streaming-attoparsec See what you think of the return type. There are various possibilities, alas.

@fosskers
Copy link

fosskers commented Mar 6, 2017

Ah, nice. I'd rather have that than the catch-all utils package. The return type seems to me to be the obvious choice, except for maybe the ambiguously named Message alias.

I'll also point out a typo / historical artifact in the docs, where your import alias is A but it's used as both A and AS in the examples.

@torgeirsh
Copy link
Author

At a quick glance it looks great, thanks! Is it possible to easily recover parseL & parsedL, or are separate implementations required like in pipes-attoparsec?

@mboes
Copy link

mboes commented May 28, 2017

👍 for splitting up this package. Over time the dependency list will just get huge. I'm thinking about contributing a connection to binary, but then if we put that in streaming-utils eventually we'll have to depend on every binary-like library (cereal etc) on Earth in the same package.

@fosskers
Copy link

Oh cool @mboes , this was you.

fosskers added a commit to haskell-streaming/streaming-attoparsec that referenced this issue Mar 23, 2018
@fosskers
Copy link

A factored-out streaming-attoparsec has been added to the haskell-streaming organization. Along the way I fixed this Either issue, so this report can be closed.

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

No branches or pull requests

4 participants