fix: measure the truncation tail in visible columns, not raw width - #298
Merged
Merged
Conversation
truncate_str takes width in visible columns (line 917 measures the input with the ansi-aware measure_text_width), but subtracted the tail budget with the ansi-blind str_width, so escape sequences in the tail were charged as printed columns. A styled marker could consume the entire budget and drop the whole input.
This file contains hidden or 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
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.
truncate_strcharges the truncation tail's ANSI escape bytes against the visible-width budget, so a styled marker eats its own budget and can consume the whole thing.The inconsistency
truncate_strmeasures its two string operands with two different helpers:str_width(:884) is rawUnicodeWidthStr::width/chars().count(); it has no idea what an escape sequence is. Line 917 defineswidthin visible columns, so lines 932 and 938 are subtracting a byte-ish count from a column count.str_widthis the right helper at:126and:957, where it is applied to a chunkAnsiCodeIteratorhas already classified as non-ANSI.:932/:938are the only places in the crate where a whole user-supplied string is measured with it.Reproduction, public API only
pad_str_withreaches it on the same footing: it measures withmeasure_text_widthat:1025and then hands thatwidthtotruncate_strat:1030.After the patch all of those come back at width 10.
The fix
Measure
tailwithmeasure_text_width, the same helpertruncate_stralready uses forsfifteen lines above.The
#[cfg(not(feature = "ansi-parsing"))]branch at:980is deliberately untouched: theremeasure_text_widthisstr_width(:133), so changing it would be pure churn.Verification
test_truncate_str_ansi_tailfails on the pristine tree (left: "\x1b[31m...\x1b[0m",right: "foo bar\x1b[31m...\x1b[0m") and passes with the patch. Mutating the fix in both directions kills it:str_width(tail)measure_text_width(tail) + 1test_truncate_str,test_truncate_str_no_ansi,test_truncate_str_multibyte_no_panic,test_pad_str,test_pad_str_with, new)tail_width = 0Repo gates, run verbatim from the
Makefile:make test(all seven feature rows) 0,make check0,make lint(cargo clippy --examples --tests --all-features -- --deny warnings) 0,make format-check0,cargo test --doc --all-features0,cargo hack check --each-feature7/7. The new test is gated onfeature = "ansi-parsing"and its assertions are pure ASCII, so thestd,ansi-parsingrow (unicode-width off) agrees with the--all-featuresrow.make check-minverneedscargo-minimal-versions, which I do not have locally; that job iscontinue-on-erroranyway.Disclosure: this patch was prepared with AI assistance. I ran the tests, the mutation matrix and every gate above myself and I stand behind the change.