Improve the docs for the write and writeln macros #40934

SamWhited · 2017-03-30T18:28:42Z

This change reduces duplication by linking the documentation for
writeln! to write!. It also restructures the write! documentation
to read in a more logical manner (I hope; feedback would be welcome).

Updates #29329, #29381

rust-highfive · 2017-03-30T18:28:49Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (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.

steveklabnik

This looks great! Just a few nits.

steveklabnik · 2017-03-30T18:59:17Z

src/libcore/macros.rs

-/// This macro accepts a 'writer' (any value with a `write_fmt` method), a format string, and a
-/// list of arguments to format.
+/// This macro accepts a format string, a list of arguments, and a 'writer'.
+/// Arguments will be formatted according to the specified format string and the result will be


without an extra /// in between, Markdown will merge these into one paragraph; could you add some extra ///s here and elsewhere in the file? I think you (rightfully) wanted these to be in different paragraphs 😄

I actually didn't (I just return after sentences out of habit so that changes to one sentence in the future don't necessarily cause the whole paragraph to be rewrapped and make the diff longer); happy to add a paragraph if you think it should be two, but it felt like a single coherent description to me (and the docs online currently are hard to read because it's mostly "single line" paragraphs, meaning there's as much whitespace in the text as text itself). Personally, I thought it was easier to read this way, but as I said, if you're sure it should be chagned I'm happy to split it out into single sentence paragraphs.

It's all good! I tend to over-whitespace. If you want it in one paragraph, can you then remove the \ns so that's more obvious? I hear you about diffs, but we haven't found it too onerous, and so don't worry about that.

Done, thanks again for the review and explaining the prefered style!

No problem 😁

steveklabnik · 2017-03-30T19:00:22Z

src/libcore/macros.rs

+/// Arguments will be formatted according to the specified format string and the result will be
+/// passed to the writer.
+/// The writer may be any value with a `write_fmt` method; generally this comes from an
+/// implementation of either the [`std::fmt::Write`][fmt_write] or the [`std::io::Write`][io_write]


I know that you're adapting existing text here, but while you're here... the current style is to do

[`std::fmt::Write`]

in the text and then

[`std::fmt::Write`]: URL

for the link. Could you change all of these to be in that style? Thank you!

done; thanks!

steveklabnik · 2017-03-30T19:00:56Z

src/libcore/macros.rs

@@ -393,30 +388,10 @@ macro_rules! write {

 /// Write formatted data into a buffer, with a newline appended.
 ///
-/// On all platforms, the newline is the LINE FEED character (`\n`/`U+000A`) alone
-/// (no additional CARRIAGE RETURN (`\r`/`U+000D`).


It looks like we're losing this bit of docs; could we keep this?

oops, I meant to move that into write, but it doesn't make sense there and I guess I forgot to put it back after not including it in write. Thanks!

steveklabnik

one last tiny ! to be removed and then this is good to go 👍

steveklabnik · 2017-03-30T19:20:55Z

src/libcore/macros.rs

-/// The `write_fmt` method usually comes from an implementation of [`std::fmt::Write`][fmt_write]
-/// or [`std::io::Write`][io_write] traits. The term 'writer' refers to an implementation of one of
-/// these two traits.
+/// See [`std::fmt!`] for more information on the format string syntax.


an extra ! snuck in here

oops, good eye; fixed. Squashing again since it's a one character change.

steveklabnik · 2017-03-30T19:21:00Z

src/libcore/macros.rs

-/// [io_write]: ../std/io/trait.Write.html
-/// [fmt_result]: ../std/fmt/type.Result.html
-/// [io_result]: ../std/io/type.Result.html
+/// [`std::fmt!`]: ../std/fmt/index.html


SamWhited · 2017-03-30T19:28:22Z

A nit on my own PR: I wonder if it would be helpful to have the statement about fmt strings (or similar):

See [std::fmt] for more information on the format string syntax.

in the writeln docs too so you don't have to click through to write, then click through to fmt when you just wanted to figure out what the format string syntax was (which is probably what 90% of people want when they read this doc, I guess)? I'm not sure that one less hop is super important, but it might be nice to click straight through to fmt if you land on writeln.

steveklabnik · 2017-03-30T19:37:56Z

I'm not sure that one less hop is super important, but it might be nice to click straight through to fmt if you land on writeln.

I think that's a great idea. You're right that it's small, but little things can add up.

SamWhited · 2017-03-30T19:39:58Z

Done ⤴

This change reduces duplication by linking the documentation for `writeln!` to `write!`. It also restructures the `write!` documentation to read in a more logical manner. Updates #29329, #29381

steveklabnik · 2017-03-30T19:57:17Z

Awesome! Thanks so much!

@bors: r+ rollup

bors · 2017-03-30T19:57:17Z

📌 Commit b03edb4 has been approved by steveklabnik

…s, r=steveklabnik Improve the docs for the write and writeln macros This change reduces duplication by linking the documentation for `writeln!` to `write!`. It also restructures the `write!` documentation to read in a more logical manner (I hope; feedback would be welcome). Updates rust-lang#29329, rust-lang#29381

Rollup of 10 pull requests - Successful merges: #40694, #40842, #40869, #40888, #40898, #40904, #40925, #40928, #40929, #40934 - Failed merges:

rust-highfive assigned aturon Mar 30, 2017

steveklabnik suggested changes Mar 30, 2017

View reviewed changes

Improve the docs for the write and writeln macros

b03edb4

This change reduces duplication by linking the documentation for `writeln!` to `write!`. It also restructures the `write!` documentation to read in a more logical manner. Updates #29329, #29381

steveklabnik approved these changes Mar 30, 2017

View reviewed changes

frewsxcv mentioned this pull request Mar 31, 2017

Rollup of 10 pull requests #40948

Closed

frewsxcv mentioned this pull request Mar 31, 2017

Rollup of 10 pull requests #40950

Merged

bors added a commit that referenced this pull request Mar 31, 2017

Auto merge of #40950 - frewsxcv:rollup, r=frewsxcv

40feadb

Rollup of 10 pull requests - Successful merges: #40694, #40842, #40869, #40888, #40898, #40904, #40925, #40928, #40929, #40934 - Failed merges:

bors merged commit b03edb4 into rust-lang:master Mar 31, 2017

SamWhited deleted the improve_write_writeln_docs branch March 31, 2017 18:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve the docs for the write and writeln macros #40934

Improve the docs for the write and writeln macros #40934

SamWhited commented Mar 30, 2017 •

edited

Loading

rust-highfive commented Mar 30, 2017

steveklabnik left a comment

steveklabnik Mar 30, 2017

SamWhited Mar 30, 2017 •

edited

Loading

steveklabnik Mar 30, 2017

SamWhited Mar 30, 2017

steveklabnik Mar 30, 2017

steveklabnik Mar 30, 2017

SamWhited Mar 30, 2017

steveklabnik Mar 30, 2017

SamWhited Mar 30, 2017

steveklabnik left a comment

steveklabnik Mar 30, 2017

SamWhited Mar 30, 2017

steveklabnik Mar 30, 2017

SamWhited commented Mar 30, 2017 •

edited

Loading

steveklabnik commented Mar 30, 2017

SamWhited commented Mar 30, 2017

steveklabnik commented Mar 30, 2017

bors commented Mar 30, 2017

Improve the docs for the write and writeln macros #40934

Improve the docs for the write and writeln macros #40934

Conversation

SamWhited commented Mar 30, 2017 • edited Loading

rust-highfive commented Mar 30, 2017

steveklabnik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SamWhited Mar 30, 2017 • edited Loading

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

Choose a reason for hiding this comment

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

SamWhited commented Mar 30, 2017 • edited Loading

steveklabnik commented Mar 30, 2017

SamWhited commented Mar 30, 2017

steveklabnik commented Mar 30, 2017

bors commented Mar 30, 2017

SamWhited commented Mar 30, 2017 •

edited

Loading

SamWhited Mar 30, 2017 •

edited

Loading

SamWhited commented Mar 30, 2017 •

edited

Loading