Add expected result for test_text_width in the case where unicode-width is enabled and strip-ansi-escapes is not. #105
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…th is enabled and strip-ansi-escapes is not.
pksunkara
approved these changes
Sep 4, 2021
dtolnay
pushed a commit
to dtolnay-contrib/console
that referenced
this pull request
Feb 4, 2022
…sole-rs#105) This is my fault, sorry --- I messed up a review suggestion for PR console-rs#104 and broke the build. This should get it compiley again.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have rust-console packaged in Debian and our automated tests for it are failing. Specifically the tests fail when tested with
cargo test --features unicode-width --no-default-features
---- utils::test_text_width stdout ----
thread 'utils::test_text_width' panicked at 'assertion failed:
(left == right)left:
17,right:
21', src/utils.rs:849:5note: run with
RUST_BACKTRACE=1environment variable to display a backtraceThe version currently in Debian unstable is 0.13.0 but I have also tested this manually with 0.14.1 and trunk and got the same result. I also looked at 0.8.0 the version currently in Debian stable and testing and discovered that the features are not optional there.
The test takes a simple English word, applies a bunch of styling to it, converts it to a string with ansi escapes and then passes it to measure_text_width, the exact behavior of measure_text_width depends on which features are enabled.
If strip-ansi-escapes is enabled then it strips the ansi escapes before measuring the length. If not then the string is passed on to the actual measurement step as-is.
If unicode-width is enabled the measurement is performed using it, otherwise a raw measurement of the string length is used.
I suspect that unicode-width is treating control characters like escape as having zero width and that is why it is producing a lower result. The test already has separate expected results depending on whether strip-ansi-escapes is enabled and I Debian I simply added a new expected result for the case where unicode-width is enabled but strip-ansi-escapes is not.