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

Add "Redirect both stdout and stderr of child process to the same fle" #349

Merged
merged 1 commit into from Oct 31, 2017

Conversation

Projects
None yet
2 participants
@ludwigpacifici
Copy link
Contributor

ludwigpacifici commented Oct 28, 2017

fixes #345

@ludwigpacifici

This comment has been minimized.

Copy link
Contributor Author

ludwigpacifici commented Oct 28, 2017

Hello @budziq

I change the command from find / (what I suggested in #345) tols. The execution time is quicker.

Also, I hardcoded the filename. I saw a crate to create temporaries, but the example would be less minimal.

Let me know your thoughts.

@budziq
Copy link
Collaborator

budziq left a comment

Nice! Just some minor suggestions below.


[![std-badge]][std] [![cat-os-badge]][cat-os]

Spawns a Unix command and redirects `stdout` and `stderr` to the same

This comment has been minimized.

@budziq

budziq Oct 30, 2017

Collaborator

The api itself is cross platform so I would avoid mentioning unix here. Would rather go with child processes.

used to reference the same file for `stdout` and `stderr`.

The below recipe is equivalent to run `ls . oops &> out.txt`.

This comment has been minimized.

@budziq

budziq Oct 30, 2017

Collaborator

Here we can mention that the example would be equivalent to following Unix shell command.

Spawns a Unix command and redirects `stdout` and `stderr` to the same
file. It follows the same idea as [run piped external
commands](#ex-run-piped-external-commands), however [`process::Stdio`]
will be constructed from a file. In addition, [`File::try_clone`] is

This comment has been minimized.

@budziq

budziq Oct 30, 2017

Collaborator

the statement might be slightly misleading. we might want to rewrite to emphasize that process::Stdio will write to provided Fileand that File::try_clone will duplicate the file handle ensuring the write pointer is synchronized for stderr and stdout

@ludwigpacifici ludwigpacifici force-pushed the ludwigpacifici:master branch from 9f8377c to 22e04eb Oct 31, 2017

@ludwigpacifici ludwigpacifici force-pushed the ludwigpacifici:master branch from 22e04eb to 479031c Oct 31, 2017

@ludwigpacifici

This comment has been minimized.

Copy link
Contributor Author

ludwigpacifici commented Oct 31, 2017

@budziq, PR updated with your comments.

I also changed changed the equivalent Unix command from ls . oops &> out.txt to
ls . oops >out.txt 2>&1. The former may not be compatible and the later is more standard.

@budziq budziq merged commit 0ecf0bf into rust-lang-nursery:master Oct 31, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@budziq

This comment has been minimized.

Copy link
Collaborator

budziq commented Oct 31, 2017

Well done!

@ludwigpacifici

This comment has been minimized.

Copy link
Contributor Author

ludwigpacifici commented Oct 31, 2017

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.