Part of https://github.com/rust-lang/rust/issues/29329
http://doc.rust-lang.org/std/process/
Here's what needs to be done to close out this issue:
abort](https://doc.rust-lang.org/std/process/fn.abort.html) needs an example, preferably showing off its destuctor behavior. (some elaboration here, and a first PR here)abort should also talk about its relationship with panic!, specifically, unwinding vs aborting panics.abort has a bug: The [abort] function terminates the process, the []s need to be dropped.Child](https://doc.rust-lang.org/std/process/struct.Child.html) the Note shouldn't be one, and should come before the examples.ChildStderr](https://doc.rust-lang.org/std/process/struct.ChildStderr.html) should have its second sentence as not a summary, needs some newlines.ChildStdin](https://doc.rust-lang.org/std/process/struct.ChildStdin.html) has the same issue.ChildStdout](https://doc.rust-lang.org/std/process/struct.ChildStdout.html) has the same issue.Command](https://doc.rust-lang.org/std/process/struct.Command.html) is good but many of its methods could use more elaboration in their docs. See below for some details.ExitStatus](https://doc.rust-lang.org/std/process/struct.ExitStatus.html) should link to status, and its code method needs an example.Output](https://doc.rust-lang.org/std/process/struct.Output.html) should link to output.Stdio](https://doc.rust-lang.org/std/process/struct.Stdio.html) should link to stdio and needs expanded explanations and examples.exit](https://doc.rust-lang.org/std/process/fn.exit.html) needs some examples to show its behavior. An example of how people use this in main would be fantastic too.With regards to Command, @frewsxcv brought up in https://github.com/rust-lang/rust/issues/40983 that
stdin, stdout, and stderr should specify what the default behavior is if they aren't explicitly called.And @retep998 mentions
Note that the default behavior depends on whether the child process is using the console subsystem or windows subsystem. Console subsystem apps will inherit your stdout/stdin/stderr while windows subsystem apps will start with no stdout/stdin/stderr unless you explicitly assign them.
This API really could use a better documentation. To list all failure cases of several functions would be great :)
I am happy to mentor anyone who wants to tackle this issue.
I'd like to help tackle this one and learn more about the Rust process module :)
I've had to muck around with this module a lot for my own libraries and things. I'll be glad to tackle a few of these to help document them for others.
@julienXX @mgattozzi wonderful! Let me know if you need help in any way.
Hi @steveklabnik I would like to try to tackle this issue but I need some mentor help (first time). Is adding this example for abort useless or "better than nothing"?
use std::process::abort;
fn main() {
println!("aborting");
abort();
}
If it's not considered useless, I could add it by creating a PR (with this code in a Examples section). Should I PR directly on master?
_Side note: I don't know right how to show off its destuctor behavior_
Hey @rap2hpoutre ! Sure thing ๐
So, this example is good as a basic one, though I might add
use std::process;
fn main() {
println!("aborting");
process::abort();
// execution never gets here
}
That is, two things:
That said, after this basic example, another one would be good as well, showing this:
Side note: I don't know right how to show off its destuctor behavior
So, the key is that destruction never get run. This is in contrast with the default panic behavior, which will. Let me make another checkbox for that, I didn't even think of it.
For something like Box<T> that manages memory, this doesn't matter, as the OS will clean up the memory itself, so that's an okay but not great example.
For something that matters more, we need something with a Drop implementation that does something else. We could go one of two ways:
Drop that prints a message of some kind.BufWriter:The buffer will be written out when the writer is dropped.
The first example might be more straightforward and clear, but the latter example might be more realistic. It might even be worth it to include both, actually.... what do you think?
A PR for one, two, or three of these examples would be great. We've already talked about the first one above, but if you're interested in these more complex ones, we can go into detail about what they should look like ๐ They also don't all have to be in the same PR.
I could add it by creating a PR (with this code in a Examples section). Should I PR directly on master?
That'd be wonderful, and yes, all PRs go to master ๐
Ok, thanks again!
Now I have a proposition for the second (2/3) example of abort (thanks to your advice):
use std::process;
struct HasDrop;
impl Drop for HasDrop {
fn drop(&mut self) {
println!("This will never be printed!");
}
}
fn main() {
let _x = HasDrop;
process::abort();
// the destructor implemented for HasDrop will never get run
}
And the description of this exemple might be:
The abort function terminates the process, so the destructor will not get run on the example below:
I post my proposition here just to be mentored one more time before sending PR (and because I think I have to wait my first PR to be merged).
EDIT: grammar fix.
I would have it print
"This will never be printed!"
as a small grammar fix. And the description should end in :, not . Other than those two tiny things, looks great!
and because I think I have to wait my first PR to be merged
You don't have to as long as you use branches in git.
@steveklabnik we should probably update the state of this issue since a few things have landed.
@mgattozzi done, thank you for the reminder!
@steveklabnik ChildStdin and ChildStdout PR was merged so we can check that off as well!
@steveklabnik Trying to get involved in this. Before I open a PR, could you give this a once-over please, it's my first contribution to Rust and Rust docs:
https://github.com/citizen428/rust/commit/ee7c170189b7c12f533814c91cbd1fa328bc47c6
Also I'm not entirely sure what "should link to status" means, can you clarify please?
@citizen428 sorry for the late response here; I took a few days off last week.
This looks great! There's only one thing I would change, and that's adding an /// # Examples line like the rest of the docs have.
Also I'm not entirely sure what "should link to status" means, can you clarify please?
Sure thing! You get an ExitStatus by calling the status method on Command; so there should be a sentence saying so, with a link. Kind of like how https://doc.rust-lang.org/std/iter/struct.Cycle.html has a link to cycle.
@steveklabnik No worries, it's good to take breaks ๐
I amended my previous commit and force pushed, if you have a sec please have a look at https://github.com/citizen428/rust/commit/351d2d7228eece349e3f3c0cddd5410eb2c7dfcf, thanks!
Looks great!
On Mon, May 15, 2017 at 1:04 PM, Michael Kohl notifications@github.com
wrote:
@steveklabnik https://github.com/steveklabnik No worries, it's good to
take breaks ๐I amended my previous commit and force pushed, if you have a sec please
have a look at 351d2d7
https://github.com/rust-lang/rust/commit/351d2d7228eece349e3f3c0cddd5410eb2c7dfcf,
thanks!โ
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rust-lang/rust/issues/29370#issuecomment-301538369,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABsiis8lBhiphTrX0PMXY4CKG2Tb93Nks5r6IWwgaJpZM4GV1x-
.
A specific example that would go a long way to making the process module more usable would be to show setting up .stdin(Stdio::piped()) and then writing to the spawned process's stdin:
The following snippet that took me far too long to write, I think if the docs had included the process.stdin... line, I'd have put the rest of the pieces together far quicker:
let mut process = Command::new("openssl")
.arg("x509")
.arg("-text")
.arg("-noout")
.arg("-inform")
.arg("der")
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.spawn()
.unwrap();
process.stdin.as_mut().unwrap().write_all(&chain[0].0).unwrap();
let out = process.wait_with_output().unwrap();
String::from_utf8_lossy(&out.stdout).into_owned()
I'd like to take a stab at the two remaining issues with abort (the extra brackets and the missing relationship to panic!).
This'll be my first attempt at a contribution - I'll fork rust just now and try to get a branch up soon.
Excellent!
Done! But before I open a PR, can I make sure I'm not misunderstanding anything?
Commits for the two changes are here:
Pirh/rust@7ab20c85
Pirh/rust@6dfa45d
Also, I mention Cargo.toml in part of the description of panic=abort. Is that OK, or do we avoid mentioning cargo concepts in the std lib documentation?
Don't worry about opening up PRs, it's actually easiest to review as a PR. WIP PRs are totally fine!
Also, I mention Cargo.toml in part of the description of panic=abort. Is that OK, or do we avoid mentioning cargo concepts in the std lib documentation?
We don't, but not due to any specific policy exactly, it's just never been directly relevant.
(Oh, and these look good to me!)
Great! I'll submit a PR just now and in future go straight ahead. Thanks for taking a look though.
Think I'm going to take a look at the Stdio links/examples.
@steveklabnik the two abort check boxes should be ticked off now that #44905 was merged
I'd like to help improve the module docs.
@Technius that'd be great! Let me know if you need any help!
The ExitStatus, Output, and Stdio can be checked off now, as well as the default behaviour of stdin/stdout/stderr.
The Command box can possibly also be checked off (at least it looks good to me, not sure what other detail is needed).
All that's left (I think) is module level documentation, and an explanation of how stdio works in different Windows subsystems.
I'm giving this issue a close. Thank you to everyone who's pitched in on these docs over the last few years. If there's still improvements to do here, please open a new issue, one per thing. Thanks!
Most helpful comment
A specific example that would go a long way to making the
processmodule more usable would be to show setting up.stdin(Stdio::piped())and then writing to the spawned process'sstdin:The following snippet that took me far too long to write, I think if the docs had included the
process.stdin...line, I'd have put the rest of the pieces together far quicker: