search

Gabriella439 / pipes

Compositional pipelines
BSD 3-Clause "New" or "Revised" License
489 stars 72 forks source link

Add mtl instances for MonadReader, MonadWriter and MonadState. #59

Closed merijn closed 11 years ago

merijn commented 11 years ago

Please double check the implementation of MonadWriter makes sense.

Gabriella439 commented 11 years ago

I made some comments directly on the commit for the MonadWriter implementation.

Also, could you add a MonadError implementation, too? You can reuse liftCatch from Pipes.Lift for that purpose.

In fact, once you are done with this I should add liftPass and liftListen to Pipes.Lift to complement the mtl implementation. Then we could just have these instances just reuse the code in Pipes.Lift.

I would avoid using liftM until Csernik finishes the benchmark suite. I suspect it might actually give better performance, but I want to know for sure before switching to liftM as idiomatic coding style for pipes.

merijn commented 11 years ago

Using liftCatch from Pipes.Lift causes an import cycle between Pipes.Internal and Pipes.Lift. I think the cleanest solution is to move liftCatchError to Pipes.Internal and then reexport from Pipes.Lift?

merijn commented 11 years ago

Ok, fixed the MonadWriter instance and added MonadError. I moved liftCatchError to Pipes.Internal to avoid a circular import, it's reexported from Pipes.Lift.

Gabriella439 commented 11 years ago

Yeah, it would have to go in Pipes.Internal to work and I'm fine with that.

I think this is pretty good. Is it alright if I go ahead and merge this in?

merijn commented 11 years ago

Yeah, as far as I'm concerned this should be it.