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

AsyncCommand use main.stdout / main.stderr #61

Closed
jakeheis opened this issue Apr 1, 2018 · 6 comments
Closed

AsyncCommand use main.stdout / main.stderr #61

jakeheis opened this issue Apr 1, 2018 · 6 comments

Comments

@jakeheis
Copy link

jakeheis commented Apr 1, 2018

Right now, AsyncCommand will always change the process's stdout and stderr to a Pipe (https://github.com/kareman/SwiftShell/blob/master/Sources/SwiftShell/Command.swift#L291).

There should be a way to start an AsyncCommand which still directs output to main.stdout and main.stderr. Simply forwarding the output using something like:

command.stdout.onOutput { $0.write(to: &stdout) }

is not sufficient, as pipes are block buffered but main.stdout is line buffered, so the command's output will not be written to stdout in the expected way.

I think this could potentially be solved by changing AsyncCommand.init to something like:

init(unlaunched process: Process, stdout: WritableStream? = nil, stderr: WritableStream? = nil)
@kareman
Copy link
Owner

kareman commented Apr 3, 2018

Yes that would be useful. But AsyncCommand.init is not public, so we need to change the public api. Do you think the use case is common enough to warrant a new function runAsyncAndPrint, which returns a superclass of AsyncCommand without stdout and stderror?

@patchthecode
Copy link

Usually one use case is enough to constantly bite you in the ass i always say 😄.
I like this library. I vote yes.

@kareman
Copy link
Owner

kareman commented Apr 4, 2018

I’m about to do some refactoring of Command.swift to move anything Linux-specific to its own file. I will implement runAsyncAndPrint after that.

Does anyone have a better idea for the name? It is a bit wordy, but I think it fits in nicely with run, runAsync and runAndPrint.

Ideally this functionality should be implemented with an extra parameter on runAsync, but the return type needs to be different.

kareman added a commit that referenced this issue Apr 9, 2018
@kareman
Copy link
Owner

kareman commented Apr 9, 2018

@jakeheis I implemented runAsyncAndPrint in branch 61-AsyncCommand_use_main.stdout.

@patchthecode
Copy link

will take pull.

@kareman
Copy link
Owner

kareman commented Apr 18, 2018

Just released version 4.1 with this feature.

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

3 participants