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 documentation to process::Child fields #3437

Merged
merged 3 commits into from Jan 16, 2021

Conversation

JOT85
Copy link
Contributor

@JOT85 JOT85 commented Jan 16, 2021

Motivation

I was using tokio::process::Child without much experience of std::process::Child and was reading the tokio docs. I worked out that I needed to use the child.stdin.take().unwrap() pattern but it would have saved me some time if it was in the docs. I later discovered that this was added to the rust std lib documentation by rust-lang/rust#74192.

Solution

Updated the doc comments on the public fields of Child to match the changes made by rust-lang/rust#74192.

Comment on lines 846 to 848
/// ```compile_fail,E0425
/// let stdin = child.stdin.take().unwrap();
/// ```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not have tests that fail to compile.

Suggested change
/// ```compile_fail,E0425
/// let stdin = child.stdin.take().unwrap();
/// ```
/// ```no_run
/// # let child: tokio::process::Child = unreachable!();
/// let stdin = child.stdin.take().unwrap();
/// ```

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the code suggested errors because the second line in unreachable, what do prefer to do? Set it to ignore rather than no_run?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just throw in some dummy command instead.

let child = tokio::process::Command::new("echo").spawn().unwrap();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Co-authored-by: Alice Ryhl <alice@ryhl.io>
@Darksonn Darksonn added A-tokio Area: The main tokio crate M-process Module: tokio/process T-docs Topic: documentation labels Jan 16, 2021
@Darksonn Darksonn merged commit 7ac44a2 into tokio-rs:master Jan 16, 2021
@Darksonn Darksonn mentioned this pull request Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-process Module: tokio/process T-docs Topic: documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants