Skip to content

Add createPipeOpen#85

Closed
ch1bo wants to merge 1 commit intofpco:masterfrom
ch1bo:master
Closed

Add createPipeOpen#85
ch1bo wants to merge 1 commit intofpco:masterfrom
ch1bo:master

Conversation

@ch1bo
Copy link

@ch1bo ch1bo commented Mar 6, 2025

This allows, for example, to read a sub-process' stderr after the process has exited.

This allows, for example, to read a sub-process' stderr after the
process has exited.
@snoyberg
Copy link
Member

snoyberg commented Mar 9, 2025

It's surprising that this is only in the internal module, does this not need to be exported elsewhere?

It would be great to have a test case to cover this, plus a minor version bump and changelog entry.

@ch1bo
Copy link
Author

ch1bo commented Mar 11, 2025

TBH I have not tried to use it through the package yet, I had this helper in my code base. Will make amends as you suggested!

@ch1bo
Copy link
Author

ch1bo commented Mar 12, 2025

I'm not sure why it did not work with createPipe originally. When trying to write tests for this:

    it "createPipe can not be read after process exit" $ do
        withProcessTerm_ (setStderr createPipe $ proc "sh" ["-c", "echo hello >&2"]) $ \p -> do
            let h = getStderr p
            waitExitCode p `shouldReturn` ExitSuccess
            S.hGetContents h `shouldReturn` "hello\n"

    it "createPipeOpen can be read after process exit" $ do
        withProcessTerm_ (setStderr createPipeOpen $ proc "sh" ["-c", "echo hello >&2"]) $ \p ->
          bracket (pure $ getStderr p) hClose $ \h -> do
            waitExitCode p `shouldReturn` ExitSuccess
            S.hGetContents h `shouldReturn` "hello\n"

Both were succeeding (I was expecting a EOF exception or so on the createPipe one).

I followed up by rewriting my workaround (using System.Process.createPipe and useHandleOpen) in terms of createPipe and it seems to do the trick for us there too.

So closing this as seemingly not needed.

@ch1bo ch1bo closed this Mar 12, 2025
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

Successfully merging this pull request may close these issues.

2 participants