Add ByteString.Lazy utilities #267
Conversation
|
Wow, thanks for doing this! Give me a day or two to review this more closely, but I approve of the overall direction of this |
src/Turtle/Bytes/Lazy.hs
Outdated
| {-| Read lazily bytes from standard input | ||
|
|
||
| -} | ||
| stdin :: Shell ByteString |
There was a problem hiding this comment.
Should this have type:
stdin :: MonadIO io => io ByteString
src/Turtle/Bytes/Lazy.hs
Outdated
|
|
||
| The chunks are not necessarily aligned to line boundaries | ||
| -} | ||
| input :: FilePath -> Shell ByteString |
There was a problem hiding this comment.
Should this have type:
input :: MonadManaged managed => FilePath -> managed ByteString|
|
||
| Throws an `ExitCode` exception if the command returns a non-zero exit code | ||
| -} | ||
| stream |
There was a problem hiding this comment.
I don't think you need any of the streaming functions like stream/streamWithErr or the functions downstream of them. The systemStrict and systemStrictWithErr versions already stream since they are using Data.ByteString.Lazy.hGetContents
|
Regarding the signature changes: yep, I forgot to transitively generalise the functions that depends on Regarding the |
|
Yeah, I agree that they don't need the |
|
One more question then, if we remove
|
|
I think you don't even Or you could rename the new |
|
I finally decided to drop the |
| -- ^ Command | ||
| -> Shell ByteString | ||
| -- ^ Chunks of bytes written to process input | ||
| -> io (ExitCode, ByteString) |
There was a problem hiding this comment.
Actually, something seems wrong here. This type implies that we have access to the ExitCode as the same time as the ByteString, which is being read lazily. If we are able to access the ExitCode, wouldn't that imply that the reading ByteString would fail due to the subprocess handle being closed?
There was a problem hiding this comment.
I guess that I should have port the initial stream version as well then... I should really try and test these functions with some examples, I'm not sure whether the out handle will be accessible or not in that case.
There was a problem hiding this comment.
One option would be to read the lazy ByteString strictly (i.e. force the entire ByteString) so that you don't have to worry about still reading it after the handle is closed
The starting point was a similar issue from the one raised in #209. Compared to the solution proposed in the thread, I'd prefer to go for a whole set of utilities for
ByteString.Lazy. It allows almost the same shell facilities than the strict version, except that chunks are meaningless here.