Improve std::process module docs #45295

Technius · 2017-10-15T02:28:13Z

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

rust-highfive · 2017-10-15T02:28:19Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @steveklabnik (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

kennytm · 2017-10-15T06:00:49Z

src/libstd/process.rs

-//! let mut child = Command::new("/bin/cat")
-//!                         .arg("file.txt")
+//! // Note that by default, the output of the command will be sent to stdout
+//! let child = Command::new("echo")


child needs to be mut.

[01:19:37] failures: [01:19:37] [01:19:37] ---- process.rs - process (line 21) stdout ---- [01:19:37] error[E0596]: cannot borrow immutable local variable `child` as mutable [01:19:37] --> process.rs:12:13 [01:19:37] | [01:19:37] 7 | let child = Command::new("echo") [01:19:37] | ----- consider changing this to `mut child` [01:19:37] ... [01:19:37] 12 | let ecode = child.wait() [01:19:37] | ^^^^^ cannot borrow mutably [01:19:37] [01:19:37] thread 'rustc' panicked at 'couldn't compile the test', /checkout/src/librustdoc/test.rs:283:12 [01:19:37] note: Run with `RUST_BACKTRACE=1` for a backtrace. [01:19:37] [01:19:37] [01:19:37] failures: [01:19:37] process.rs - process (line 21) [01:19:37] [01:19:37] test result: FAILED. 877 passed; 1 failed; 10 ignored; 0 measured; 0 filtered out

steveklabnik

This looks good, but a few questions. Also travis is failing for some reason.

steveklabnik · 2017-10-17T14:42:08Z

src/libstd/process.rs

+//!
+//! TODO
+//!
+//! # Examples


why is this moved down? The above is still an example 😄

Good point! I planned on having a brief overview of the entire module first and then show some additional examples. Maybe I should rename the "Example" section to "Additional Examples"?

I just figured out that I could just fold the examples into the section describing Stdio. I'll be updating the PR soon.

steveklabnik · 2017-10-17T14:42:23Z

src/libstd/process.rs

+//!
+//! # Handling I/O
+//!
+//! TODO


do you intend on filling this out in this PR? If not, I'd remove this section.

Yes, I plan on adding a few demonstrations of Stdio here. Incidentally, the TODO is causing travis to fail.

Technius · 2017-10-18T03:48:11Z

I think I've covered almost everything. If there's something else that I should mention in the docs, let me know!

carols10cents · 2017-10-23T15:15:19Z

Looks like this is back in @steveklabnik's court-- I think he's at conferences this week though :)

steveklabnik · 2017-10-27T13:37:19Z

Yup, sorry that it took me some time. Looks great, thanks a ton!

@bors: r+ rollup

bors · 2017-10-27T13:37:21Z

📌 Commit 3566832 has been approved by steveklabnik

@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

kennytm · 2017-10-28T07:28:10Z

@bors r-

The example failed to run on Windows.

[01:31:48] ---- process.rs - process (line 76) stdout ----
[01:31:48] 	thread 'rustc' panicked at 'test executable failed:
[01:31:48] 
[01:31:48] thread 'main' panicked at 'failed to execute child: Error { repr: Os { code: 2, message: "The system cannot find the file specified." } }', src\libcore\result.rs:906:4
[01:31:48] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:31:48] 
[01:31:48] ', src\librustdoc\test.rs:323:16
[01:31:48] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:31:48] 
[01:31:48] 
[01:31:48] failures:
[01:31:48]     process.rs - process (line 76)

kennytm · 2017-10-28T07:29:05Z

src/libstd/process.rs

+//! Note that [`ChildStderr`] and [`ChildStdout`] implement [`Write`] and
+//! [`ChildStdin`] implements [`Read`]:
+//!
+//! ```


Windows does not have /bin/cat. Could you keep it ```no_run or change to another program for this example?

I added no_run to the example involving cat and the example involving sed.

kennytm · 2017-10-29T06:24:15Z

@bors r=steveklabnik

bors · 2017-10-29T06:24:16Z

📌 Commit 84ab6ae has been approved by steveklabnik

bors · 2017-10-29T06:24:26Z

⌛ Testing commit 84ab6ae with merge 7d475a2...

@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

bors · 2017-10-29T08:59:16Z

☀️ Test successful - status-appveyor, status-travis
Approved by: steveklabnik
Pushing 7d475a2 to master...

Add a brief description and two examples to std::process

5243a98

rust-highfive assigned steveklabnik Oct 15, 2017

Fixed accidental deletion of colon

e788e90

kennytm reviewed Oct 15, 2017

View reviewed changes

kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 15, 2017

Technius added 2 commits October 15, 2017 13:11

Fix std::process hello world example

bb74b13

Create section on how to spawn processes; change module description

f67f662

steveklabnik reviewed Oct 17, 2017

View reviewed changes

Add child process IO handling docs

3566832

Technius changed the title ~~[WIP] Improve std::process module docs~~ Improve std::process module docs Oct 18, 2017

steveklabnik approved these changes Oct 27, 2017

View reviewed changes

kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 27, 2017

frewsxcv mentioned this pull request Oct 28, 2017

Rollup of 7 pull requests #45586

Closed

kennytm reviewed Oct 28, 2017

View reviewed changes

kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 28, 2017

Add no_run to process examples involving unix commands

84ab6ae

kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 29, 2017

bors merged commit 84ab6ae into rust-lang:master Oct 29, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve std::process module docs #45295

Improve std::process module docs #45295

Technius commented Oct 15, 2017

rust-highfive commented Oct 15, 2017

kennytm Oct 15, 2017

steveklabnik left a comment

steveklabnik Oct 17, 2017

Technius Oct 17, 2017

Technius Oct 18, 2017

steveklabnik Oct 17, 2017

Technius Oct 17, 2017

Technius commented Oct 18, 2017

carols10cents commented Oct 23, 2017

steveklabnik commented Oct 27, 2017

bors commented Oct 27, 2017

kennytm commented Oct 28, 2017

kennytm Oct 28, 2017

Technius Oct 29, 2017

kennytm commented Oct 29, 2017

bors commented Oct 29, 2017

bors commented Oct 29, 2017

bors commented Oct 29, 2017

Improve std::process module docs #45295

Improve std::process module docs #45295

Conversation

Technius commented Oct 15, 2017

rust-highfive commented Oct 15, 2017

Choose a reason for hiding this comment

steveklabnik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Technius commented Oct 18, 2017

carols10cents commented Oct 23, 2017

steveklabnik commented Oct 27, 2017

bors commented Oct 27, 2017

kennytm commented Oct 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kennytm commented Oct 29, 2017

bors commented Oct 29, 2017

bors commented Oct 29, 2017

bors commented Oct 29, 2017