Is BufferWriter::print expected to flush?

[nissaofthesea]

(My triple is x86_64-unknown-linux-gnu and I'm using termcolor 1.2.0 with rustc 1.67.1)

In this example, I'm writing some text that is not postfixed by a newline to a Buffer and calling BufferWriter::print with it.
I expect that this will flush the stream before reading from stdin, but it doesn't appear to.

use std::io::{self, BufRead, Write};
use termcolor::{BufferWriter, ColorChoice};
fn main() -> io::Result<()> {
    let stdout = BufferWriter::stdout(ColorChoice::Never);
    let mut buffer = stdout.buffer();
    buffer.write(b"text w/o newline")?;
    stdout.print(&buffer)?;
    io::stdin().lock().read_line(&mut String::new())?;
    Ok(())
}

It works as expected if I add a newline to the string literal (similar to std line buffering).
And it's also the same if termcolor structures are swapped out for an std handle to the stream and a call to write_all.

The documentation for Buffer::print doesn't say anything about flushing or buffering, so the way I'm interpreting it is as a full unconditional write & flush of the buffer.
If this is incorrect, I would also expect calling flush on the Buffer to fix this, but it doesn't appear to.

use std::io::{self, BufRead, Write};
use termcolor::{BufferWriter, ColorChoice};
fn main() -> io::Result<()> {
    let stdout = BufferWriter::stdout(ColorChoice::Never);
    let mut buffer = stdout.buffer();
    buffer.write(b"text w/o newline")?;
    stdout.print(&buffer)?;
    buffer.flush()?; // now we flush here, still the same
    io::stdin().lock().read_line(&mut String::new())?;
    Ok(())
}

Can you replicate this, and if so is this a bug?
Thanks!

EDIT: I can flush streams manually but I don't know what state termcolor maintains that might be disrupted, and if that would be correct behavior on all platforms.

[BurntSushi]

Yeah the docs here indeed seem quite poor. I had to look at the implementation to figure things out here. But basically, BufferWriter uses line buffering by virtue of using std::io::stdout() through StandardStreamType::Stdout. I think that's the right default. It looks like it internally could use StandardStreamType::StdoutBuffered too. The latter buffers writes up to some block size and then flushes, where as the former uses a line oriented buffering strategy. It only flushes once a \n is written.

I think when I originally made the termcolor API, there was no BufferedStandardStream` and so everything was just line buffered.

But either way, print should document what it does and I believe it should not flush. Then, we should add a new flush method on BufferWriter since BufferWriter is the thing that owns the underlying writer. I believe you are technically correct that you can work-around this by calling std::io::stdout().flush(), but that breaks the abstraction boundary.

[nissaofthesea]

I agree with adding BufferWriter::flush and documenting that print doesn't flush.

It might also be worth documenting that flush in the Write implementation of Buffer is a no-op, and with the proposed change, should not be confused with BufferWriter::flush.
Looking at the implementation, it appears to be a no-op: flushing the Vec<u8> or for WindowsBuffer returning Ok(()).

[BurntSushi]

It might also be worth documenting that flush in the Write implementation of Buffer is a no-op, and with the proposed change, should not be confused with BufferWriter::flush.
Looking at the implementation, it appears to be a no-op: flushing the Vec<u8> or for WindowsBuffer returning Ok(()).

Yeah that would be fine. Buffer is just an in memory buffer, so flush is meaningless there. Same with the std::io::Write impl for Vec<u8>: https://doc.rust-lang.org/src/std/io/impls.rs.html#381-413