-
Notifications
You must be signed in to change notification settings - Fork 245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix subtract with overflow when measuring terminal line length #547
Conversation
Sorry for the regression, and thanks for the report and fix! I'm going to merge and release this -- it would be great if you are able to add a test that covers this behavior somehow. |
Agreed! I did try to create a test case, but didn't get it to fail properly yet, which is weird 🙃. I expected that I'll have another try tonight. |
tests/multi-autodrop.rs
Outdated
@@ -12,6 +12,8 @@ fn main() { | |||
}; | |||
|
|||
{ | |||
// TODO(clippy false positive): https://github.com/rust-lang/rust-clippy/issues/10577 | |||
#[allow(clippy::redundant_clone)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this because otherwise the Clippy lint workflow will fail.
I added a test 🙂. |
If measure_text_width returns 0, the line is effectively empty, although line.is_empty() may still be false. This can for example happen when there is a line which just consists of ANSI color escape sequences.
I removed the commit which added the same as #549, and rebased on main. |
Great work, thanks! |
If console::measure_text_width returns 0, the line is effectively empty, although line.is_empty() may still be false. This can for example happen when there is a line which just consists of ANSI color escape sequences.
I hope I didn't undo the work done in #533.
An alternative would be to do:
closes #546