Skip to content

Commit 76619b9

Browse files
committed
[ty] Fix rendering of long lines that are indented with tabs
It turns out that `annotate-snippets` doesn't do a great job of consistently handling tabs. The intent of the implementation is clearly to expand tabs into 4 ASCII whitespace characters. But there are a few places where the column computation wasn't taking this expansion into account. In particular, the `unicode-width` crate returns `None` for a `\t` input, and `annotate-snippets` would in turn treat this as either zero columns or one column. Both are wrong. In patching this, it caused one of the existing `annotate-snippets` tests to fail. I spent a fair bit of time on it trying to fix it before coming to the conclusion that the test itself was wrong. In particular, the annotation ranges are 4 bytes off. However, when the range was wrong, the buggy code was rendering the example as intended since `\t` characters were treated as taking up zero columns of space. Now that they are correctly computed as taking up 4 columns of space, the offsets of the test needed to be adjusted. Fixes #670
1 parent 6e25cfb commit 76619b9

File tree

4 files changed

+19
-10
lines changed

4 files changed

+19
-10
lines changed

crates/ruff_annotate_snippets/src/renderer/display_list.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ impl DisplaySet<'_> {
352352
// FIXME: `unicode_width` sometimes disagrees with terminals on how wide a `char`
353353
// is. For now, just accept that sometimes the code line will be longer than
354354
// desired.
355-
let next = unicode_width::UnicodeWidthChar::width(ch).unwrap_or(1);
355+
let next = char_width(ch).unwrap_or(1);
356356
if taken + next > right - left {
357357
was_cut_right = true;
358358
break;
@@ -377,7 +377,7 @@ impl DisplaySet<'_> {
377377
let left: usize = text
378378
.chars()
379379
.take(left)
380-
.map(|ch| unicode_width::UnicodeWidthChar::width(ch).unwrap_or(1))
380+
.map(|ch| char_width(ch).unwrap_or(1))
381381
.sum();
382382

383383
let mut annotations = annotations.clone();
@@ -1393,6 +1393,7 @@ fn format_body<'m>(
13931393
let line_length: usize = line.len();
13941394
let line_range = (current_index, current_index + line_length);
13951395
let end_line_size = end_line.len();
1396+
13961397
body.push(DisplayLine::Source {
13971398
lineno: Some(current_line),
13981399
inline_marks: vec![],
@@ -1452,12 +1453,12 @@ fn format_body<'m>(
14521453
let annotation_start_col = line
14531454
[0..(start - line_start_index).min(line_length)]
14541455
.chars()
1455-
.map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0))
1456+
.map(|c| char_width(c).unwrap_or(0))
14561457
.sum::<usize>();
14571458
let mut annotation_end_col = line
14581459
[0..(end - line_start_index).min(line_length)]
14591460
.chars()
1460-
.map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0))
1461+
.map(|c| char_width(c).unwrap_or(0))
14611462
.sum::<usize>();
14621463
if annotation_start_col == annotation_end_col {
14631464
// At least highlight something
@@ -1499,7 +1500,7 @@ fn format_body<'m>(
14991500
let annotation_start_col = line
15001501
[0..(start - line_start_index).min(line_length)]
15011502
.chars()
1502-
.map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0))
1503+
.map(|c| char_width(c).unwrap_or(0))
15031504
.sum::<usize>();
15041505
let annotation_end_col = annotation_start_col + 1;
15051506

@@ -1558,7 +1559,7 @@ fn format_body<'m>(
15581559
{
15591560
let end_mark = line[0..(end - line_start_index).min(line_length)]
15601561
.chars()
1561-
.map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0))
1562+
.map(|c| char_width(c).unwrap_or(0))
15621563
.sum::<usize>()
15631564
.saturating_sub(1);
15641565
// If the annotation ends on a line-end character, we
@@ -1754,3 +1755,11 @@ fn format_inline_marks(
17541755
}
17551756
Ok(())
17561757
}
1758+
1759+
fn char_width(c: char) -> Option<usize> {
1760+
if c == '\t' {
1761+
Some(4)
1762+
} else {
1763+
unicode_width::UnicodeWidthChar::width(c)
1764+
}
1765+
}

crates/ruff_annotate_snippets/tests/fixtures/color/regression_leading_tab_long_line.svg

Lines changed: 1 addition & 1 deletion
Loading

crates/ruff_annotate_snippets/tests/fixtures/color/strip_line_non_ws.svg

Lines changed: 1 addition & 1 deletion
Loading

crates/ruff_annotate_snippets/tests/fixtures/color/strip_line_non_ws.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,12 @@ origin = "$DIR/non-whitespace-trimming.rs"
1313
[[message.snippets.annotations]]
1414
label = "expected `()`, found integer"
1515
level = "Error"
16-
range = [241, 243]
16+
range = [237, 239]
1717

1818
[[message.snippets.annotations]]
1919
label = "expected due to this"
2020
level = "Error"
21-
range = [236, 238]
21+
range = [232, 234]
2222

2323

2424
[renderer]

0 commit comments

Comments
 (0)