Clear the rest of line after drawing to the terminal

[ChrisDenton]

EDIT: Updated to avoid changing the TermLike trait.

This will help fix rust-lang/rustup#4808

[djc]

Could we add a default implementation to keep this semver-compatible?

[ChrisDenton]

Sure. Although the only compatible default would be a no-op, which may be slightly surprising.

[djc]

Sure. Although the only compatible default would be a no-op, which may be slightly surprising.

What would the behavior be for downstream impls that get the default impl?

[ChrisDenton]

The behaviour for downstream impls that use the default will be the same as in the current version (which doesn't clear the end of the line). The only difference is if they make use of the TermLike trait themselves and then only if they use the new method and expect it to work.

Btw, in the future I would love it if move_cursor was set to true by default (or always be true and remove the option) but that would likely warrant a breaking version change (and some confidence that we've found all the bugs).

[djc]

The behaviour for downstream impls that use the default will be the same as in the current version (which doesn't clear the end of the line). The only difference is if they make use of the TermLike trait themselves and then only if they use the new method and expect it to work.

Yes, I meant, how would downstreams that have their own TermLike impl would be impacted?

Btw, in the future I would love it if move_cursor was set to true by default (or always be true and remove the option) but that would likely warrant a breaking version change (and some confidence that we've found all the bugs).

See discussion here: #759 (comment). In my mind the cost to doing semver-incompatible releases in this crate is relatively high, so if at all possible I'd really prefer to improve it without making semver-incompatible changes.

[ChrisDenton]

Yes, I meant, how would downstreams that have their own TermLike impl would be impacted?

The only place this crate makes use of the implementation is in this PR where the default no-op implementation would be the same as the old behaviour. I.e. if move_cursor is set to true they will not have this bug fix but it will otherwise work.

[djc]

Why is this the right thing to be doing? clear_chars() is documented to "[c]lear the last n characters of the current line", why does passing 0 here have the desired effect? Maybe add a comment to explain this a little?

[ChrisDenton]

After thinking about this some more, and considering the desire to not have a breaking release, I think this PR should be changed to do this unconditionally:

indicatif/src/draw_target.rs

Lines 581 to 582 in 3b0294a

	let last_line_filler = line_height.as_usize() * term_width - line.console_width();
	term.write_str(&" ".repeat(last_line_filler))?;

That would entirely avoid the need for changing TermLike. It does move the cursor to the end but that's fine so long as we also avoid writing a new line character on the next write. So remove this:

indicatif/src/draw_target.rs

Lines 572 to 574 in 3b0294a

	if idx != 0 {
	term.write_line("")?;
	}

[djc]

Sounds good!

[ChrisDenton]

Ok, done and also tested via rustup's use of this crate.

[djc]

Good stuff, thanks!