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

std::process::Command misses a way to get back stdin, stdout, stderr #32394

Closed
llogiq opened this issue Mar 21, 2016 · 5 comments
Closed

std::process::Command misses a way to get back stdin, stdout, stderr #32394

llogiq opened this issue Mar 21, 2016 · 5 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@llogiq
Copy link
Contributor

llogiq commented Mar 21, 2016

I'm not sure if this requires a RFC, but we have the StdIos within the Command, and it seems prudent not to waste them when it is dropped. So how about a function to decompose the Command into its IOs?

The following code would work (in std::process::Command):

    fn into_stdio(self) -> (Option<StdIo>, Option<StdIo>, Option<StdIo>) {
         let Command { stdin: in, stdout: out, stderr: err, .. } = self;
         (in, out, err)
    }
@oconnor663
Copy link
Contributor

What's the reason that the stdin() etc. functions need to take their argument by value? Could it have been possible to take them by reference or something like Cow (Stdio isn't ToOwned, but anyway some option to pass either value or reference)? Not that it would make sense to dramatically change the API now, just for my understanding.

@llogiq
Copy link
Contributor Author

llogiq commented Mar 21, 2016

Not taking them by value would mean the function setting the Command up would need to own them somewhere, which would pessimize ergonomics. Using something Cow-like would be great in terms of ergonomics, bit comes at a (small) runtime cost we'd have to amortize. Also I don't see a big win over what I've proposed: In most cases you don't need the streams after the fact.

@oconnor663
Copy link
Contributor

What I'd like to do maybe in theory is to write functions like this:

fn echo_foo_into_pipe(pipe: &SomePipeObject) {
    let command = Command::new("echo");
    // ...have it talk to the pipe somehow
}

The idea here is that by taking a shared reference to the pipe, the function could promise not to do anything destructive like closing it. The SomePipeObject class could be designed to require self by value to close, or something like that. But as long as Command insists on having permission to close the pipes it uses, I think we'd have to write a function more like this:

// Promises to return the argument when it's done. But *does it*!?
fn echo_foo_into_pipe(pipe: SomePipeObject) -> SomePipeObject { ... }

The downside of this to me, besides being a little more annoying to use, is that it's no longer possible to define SomePipeObject in such a way that this function's signature proves its good behavior. The function might actually close the pipe it's given and then return some new thing.

This is coming up for me because I want to spawn several shell commands talking to the same pipe. At the end of the day, it will be perfectly fine for me to build each Command, run it, and then dissemble it to build the next one. But maybe if I was writing a larger program, and I had several nested functions that needed to plumb that pipe return value back out, that would get more annoying? (Maybe I'm letting the perfect be the enemy of the good here :)

@llogiq
Copy link
Contributor Author

llogiq commented Mar 21, 2016

Please note that Command does nothing explicitly to its stdin/stdout/stderr fields, they are just dropped when the Command is. Also I think you'll need at least a mutable reference to StdIos (if not by value) to do something with them. So this would require an even more extensive redesign of the library and probably break every code in existence that uses the current setup.

@steveklabnik steveklabnik added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed A-libs labels Mar 24, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 24, 2017
@dtolnay
Copy link
Member

dtolnay commented Nov 19, 2017

Seems reasonable. Let's track this under #44434, the more general issue of extracting fields from Command.

@dtolnay dtolnay closed this as completed Nov 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants