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

API Docs: process #29370

Open
steveklabnik opened this Issue Oct 26, 2015 · 29 comments

Comments

Projects
None yet
@steveklabnik
Member

steveklabnik commented Oct 26, 2015

Part of #29329

http://doc.rust-lang.org/std/process/

Here's what needs to be done to close out this issue:

  • the module docs are okay but could use elaboration.
  • abort 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 the Note shouldn't be one, and should come before the examples.
  • ChildStderr should have its second sentence as not a summary, needs some newlines.
  • ChildStdin has the same issue.
  • ChildStdout has the same issue.
  • Command is good but many of its methods could use more elaboration in their docs. See below for some details.
  • ExitStatus should link to status, and its code method needs an example.
  • Output should link to output.
  • Stdio should link to stdio and needs expanded explanations and examples.
  • exit 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 #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.

@LukasKalbertodt

This comment has been minimized.

Contributor

LukasKalbertodt commented Jan 30, 2016

This API really could use a better documentation. To list all failure cases of several functions would be great :)

steveklabnik added a commit to steveklabnik/rust that referenced this issue May 5, 2016

steveklabnik added a commit to steveklabnik/rust that referenced this issue May 5, 2016

steveklabnik added a commit to steveklabnik/rust that referenced this issue May 5, 2016

steveklabnik added a commit to steveklabnik/rust that referenced this issue May 5, 2016

steveklabnik added a commit to steveklabnik/rust that referenced this issue May 6, 2016

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue May 7, 2016

steveklabnik added a commit to steveklabnik/rust that referenced this issue May 7, 2016

steveklabnik added a commit to steveklabnik/rust that referenced this issue May 7, 2016

@steveklabnik

This comment has been minimized.

Member

steveklabnik commented Mar 8, 2017

I am happy to mentor anyone who wants to tackle this issue.

@julienXX

This comment has been minimized.

julienXX commented Mar 24, 2017

I'd like to help tackle this one and learn more about the Rust process module :)

@mgattozzi

This comment has been minimized.

Member

mgattozzi commented Mar 25, 2017

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.

mgattozzi added a commit to mgattozzi/rust that referenced this issue Mar 25, 2017

Update `Child` docs to not have a note section
In rust-lang#29370 it's noted that for "the Note shouldn't be one, and should come before
the examples." This commit changes the positioning of the section and removes
wording that said take note in order for it to flow better with the surrounding
text and it's new position.
@steveklabnik

This comment has been minimized.

Member

steveklabnik commented Mar 25, 2017

@julienXX @mgattozzi wonderful! Let me know if you need help in any way.

mgattozzi added a commit to mgattozzi/rust that referenced this issue Mar 25, 2017

Update ChildStderr docs to be clearer
Before the docs only had a line about where it was found and that it was
a handle to stderr. This commit changes it so that the summary second line is
removed and that it's a bit clearer about what can be done with it. Part of
\rust-lang#29370
@rap2hpoutre

This comment has been minimized.

Contributor

rap2hpoutre commented Mar 29, 2017

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

@steveklabnik

This comment has been minimized.

Member

steveklabnik commented Mar 29, 2017

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:

  1. It's considered idiomatic to import the module, and then namespace the call
  2. an extra comment mentioning explicitly that the program is over at this point.

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:

  1. create a new structure that implements Drop that prints a message of some kind.
  2. find another structure in the standard library with more interesting Drop behavior. A good example might be 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 😄

@Mistodon

This comment has been minimized.

Contributor

Mistodon commented Sep 27, 2017

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.

@steveklabnik

This comment has been minimized.

Member

steveklabnik commented Sep 27, 2017

Excellent!

Mistodon pushed a commit to Mistodon/rust that referenced this issue Sep 27, 2017

Mistodon pushed a commit to Mistodon/rust that referenced this issue Sep 27, 2017

@Mistodon

This comment has been minimized.

Contributor

Mistodon commented Sep 27, 2017

Done! But before I open a PR, can I make sure I'm not misunderstanding anything?

Commits for the two changes are here:
Mistodon/rust@7ab20c8
Mistodon/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?

@steveklabnik

This comment has been minimized.

Member

steveklabnik commented Sep 28, 2017

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.

@steveklabnik

This comment has been minimized.

Member

steveklabnik commented Sep 28, 2017

(Oh, and these look good to me!)

@Mistodon

This comment has been minimized.

Contributor

Mistodon commented Sep 28, 2017

Great! I'll submit a PR just now and in future go straight ahead. Thanks for taking a look though.

bors added a commit that referenced this issue Oct 4, 2017

Auto merge of #44905 - Pirh:process_abort_docs, r=steveklabnik
Update docs for process::abort

Remove a typo and explain the relationship to `panic!`.

Part of #29370

r? @steveklabnik
@Mistodon

This comment has been minimized.

Contributor

Mistodon commented Oct 8, 2017

Think I'm going to take a look at the Stdio links/examples.

@mgattozzi

This comment has been minimized.

Member

mgattozzi commented Oct 8, 2017

@steveklabnik the two abort check boxes should be ticked off now that #44905 was merged

Mistodon pushed a commit to Mistodon/rust that referenced this issue Oct 8, 2017

kennytm added a commit to kennytm/rust that referenced this issue Oct 9, 2017

Rollup merge of rust-lang#45106 - Pirh:process_stdio_docs, r=dtolnay
Add links and examples for std::process::Stdio

As per rust-lang#29370

kennytm added a commit to kennytm/rust that referenced this issue Oct 9, 2017

Rollup merge of rust-lang#45106 - Pirh:process_stdio_docs, r=dtolnay
Add links and examples for std::process::Stdio

As per rust-lang#29370

Badel2 added a commit to Badel2/rust that referenced this issue Oct 12, 2017

@Technius

This comment has been minimized.

Contributor

Technius commented Oct 13, 2017

I'd like to help improve the module docs.

@steveklabnik

This comment has been minimized.

Member

steveklabnik commented Oct 13, 2017

@Technius that'd be great! Let me know if you need any help!

kennytm added a commit to kennytm/rust that referenced this issue Oct 14, 2017

Rollup merge of rust-lang#45113 - Pirh:process_output_links, r=stevek…
…labnik

Link std::process::Output to Command and Child

As per rust-lang#29370

kennytm added a commit to kennytm/rust that referenced this issue Oct 15, 2017

Rollup merge of rust-lang#45113 - Pirh:process_output_links, r=stevek…
…labnik

Link std::process::Output to Command and Child

As per rust-lang#29370

kennytm added a commit to kennytm/rust that referenced this issue Oct 17, 2017

Rollup merge of rust-lang#45151 - Pirh:stdio_default_docs, r=frewsxcv
Document defaults for stdin, stdout, and stderr methods of Command

For rust-lang#29370
@Mistodon

This comment has been minimized.

Contributor

Mistodon commented Oct 17, 2017

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.

frewsxcv added a commit to frewsxcv/rust that referenced this issue Oct 28, 2017

Rollup merge of rust-lang#45295 - Technius:docs/process, r=steveklabnik
Improve std::process module docs

Addresses part of rust-lang#29370

I've changed the first `cat` example to a "Hello World" example involving echo, and I've also added another example showing how to pipe output. I'm still working on the module-level description.

For now, I'd like feedback on the examples.

r? @steveklabnik

bors added a commit that referenced this issue Oct 29, 2017

Auto merge of #45295 - Technius:docs/process, r=steveklabnik
Improve std::process module docs

Addresses part of #29370

I've changed the first `cat` example to a "Hello World" example involving echo, and I've also added another example showing how to pipe output. I'm still working on the module-level description.

For now, I'd like feedback on the examples.

r? @steveklabnik
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment