Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

std: Add a note about the print! macro and output buffering #23826

Merged
merged 1 commit into from
Mar 31, 2015

Conversation

richo
Copy link
Contributor

@richo richo commented Mar 29, 2015

cc #23818

@rust-highfive
Copy link
Contributor

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

@richo richo force-pushed the note-print-macro branch from e0eb5fc to 55a6a9b Compare March 29, 2015 06:51
@@ -64,6 +64,9 @@ macro_rules! panic {
///
/// Equivalent to the `println!` macro except that a newline is not printed at
/// the end of the message.
///
/// Note that because stdout is by default unbuffered on unix systems, this may
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be buffered instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Err, yeah. Thanks for the catch!

@richo richo force-pushed the note-print-macro branch from 55a6a9b to 013b40b Compare March 29, 2015 07:13
@tanadeau
Copy link
Contributor

It would be helpful if this had a reference to the flush() method on the Write trait and an example of the use of print! that needs flushing (like a prompt).

@@ -64,6 +64,9 @@ macro_rules! panic {
///
/// Equivalent to the `println!` macro except that a newline is not printed at
/// the end of the message.
///
/// Note that because stdout is by default buffered on unix systems, this may
/// result in output not being emitted until stdout is flushed.
Copy link
Member

Choose a reason for hiding this comment

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

I think this may want to be rephrased to not mention unix and also mention "line buffering" somewhere, perhaps:

Note that stdout is frequently line-buffered by default so it may be necessary to use io::stdout().flush() to ensure the output is emitted immediately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally reasonable, is there markup to make it actually linkify io::stdout ? (Will it do this by default? That would be amazing)

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, no, there is no default auto-linking, but you can always do it manually via [foo](../bar/baz.html)

@richo richo force-pushed the note-print-macro branch from 013b40b to 28d9c11 Compare March 30, 2015 17:10
@richo
Copy link
Contributor Author

richo commented Mar 30, 2015

I updated the doc comment. Including the link inline made it no longer possible to just copy the invocation out of the comment, which I felt outweighed the benefit.

@@ -64,6 +64,9 @@ macro_rules! panic {
///
/// Equivalent to the `println!` macro except that a newline is not printed at
/// the end of the message.
///
/// Note that stdout is frequently line-buffered by default so it may be necessary to use
/// io::stdout().flush() to ensure the output is emitted immediately.
Copy link
Member

Choose a reason for hiding this comment

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

Can you match the surrounding style and wrap this to 80-characters as well? Also, can you put io::stdout().flush() in backticks as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, apologies somehow tw got screwed up in my editor.

@richo richo force-pushed the note-print-macro branch from 28d9c11 to 6e8693b Compare March 30, 2015 20:55
@alexcrichton
Copy link
Member

@bors: r+ 6e8693b rollup

Thanks!

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 31, 2015
bors added a commit that referenced this pull request Mar 31, 2015
@bors bors merged commit 6e8693b into rust-lang:master Mar 31, 2015
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants