Skip to content

Commit 87fded1

Browse files
WaffleLapkinWaffleLapkin
committed
Improve suggestions when its parts are far from each other
Previously we only show at most 6 lines of suggestions and, if the suggestions are more than 6 lines apart, we've just showed ... at the end. This is probably fine, but quite confusing in my opinion. This commit is an attempt to show ... in places where there is nothing to suggest instead, for example: Before: ```text help: consider enclosing expression in a block | 3 ~ 'l: { match () { () => break 'l, 4 | 5 | 6 | 7 | 8 | ... ``` After: ```text help: consider enclosing expression in a block | 3 ~ 'l: { match () { () => break 'l, 4 | ... 31| 32~ } }; | ```
1 parent 260c5fd commit 87fded1

File tree

1 file changed

+99
-68
lines changed

1 file changed

+99
-68
lines changed

‎compiler/rustc_errors/src/emitter.rs‎

Lines changed: 99 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -656,11 +656,6 @@ impl Emitter for SilentEmitter {
656656
}
657657
}
658658

659-
/// Maximum number of lines we will print for a multiline suggestion; arbitrary.
660-
///
661-
/// This should be replaced with a more involved mechanism to output multiline suggestions that
662-
/// more closely mimics the regular diagnostic output, where irrelevant code lines are elided.
663-
pub const MAX_SUGGESTION_HIGHLIGHT_LINES: usize = 6;
664659
/// Maximum number of suggestions to be shown
665660
///
666661
/// Arbitrary, but taken from trait import suggestion limit
@@ -1839,79 +1834,115 @@ impl EmitterWriter {
18391834
}
18401835
row_num += line_end - line_start;
18411836
}
1842-
for (line_pos, (line, highlight_parts)) in
1843-
lines.by_ref().zip(highlights).take(MAX_SUGGESTION_HIGHLIGHT_LINES).enumerate()
1844-
{
1845-
// Print the span column to avoid confusion
1846-
buffer.puts(
1847-
row_num,
1848-
0,
1849-
&self.maybe_anonymized(line_start + line_pos),
1850-
Style::LineNumber,
1851-
);
1852-
if let DisplaySuggestion::Diff = show_code_change {
1853-
// Add the line number for both addition and removal to drive the point home.
1854-
//
1855-
// N - fn foo<A: T>(bar: A) {
1856-
// N + fn foo(bar: impl T) {
1837+
let mut unhighlighted_lines = Vec::new();
1838+
for (line_pos, (line, highlight_parts)) in lines.by_ref().zip(highlights).enumerate() {
1839+
debug!(%line_pos, %line, ?highlight_parts);
1840+
1841+
let print_line = |line_pos: usize,
1842+
line: &str,
1843+
highlight_parts: &Vec<SubstitutionHighlight>,
1844+
buffer: &mut StyledBuffer,
1845+
row_num: &mut usize| {
1846+
// Print the span column to avoid confusion
18571847
buffer.puts(
1858-
row_num - 1,
1848+
*row_num,
18591849
0,
18601850
&self.maybe_anonymized(line_start + line_pos),
18611851
Style::LineNumber,
18621852
);
1863-
buffer.puts(row_num - 1, max_line_num_len + 1, "- ", Style::Removal);
1864-
buffer.puts(
1865-
row_num - 1,
1866-
max_line_num_len + 3,
1867-
&normalize_whitespace(
1868-
&*file_lines
1869-
.file
1870-
.get_line(file_lines.lines[line_pos].line_index)
1871-
.unwrap(),
1872-
),
1873-
Style::NoStyle,
1874-
);
1875-
buffer.puts(row_num, max_line_num_len + 1, "+ ", Style::Addition);
1876-
} else if is_multiline {
1877-
match &highlight_parts[..] {
1878-
[SubstitutionHighlight { start: 0, end }] if *end == line.len() => {
1879-
buffer.puts(row_num, max_line_num_len + 1, "+ ", Style::Addition);
1880-
}
1881-
[] => {
1882-
draw_col_separator(&mut buffer, row_num, max_line_num_len + 1);
1883-
}
1884-
_ => {
1885-
buffer.puts(row_num, max_line_num_len + 1, "~ ", Style::Addition);
1853+
if let DisplaySuggestion::Diff = show_code_change {
1854+
// Add the line number for both addition and removal to drive the point home.
1855+
//
1856+
// N - fn foo<A: T>(bar: A) {
1857+
// N + fn foo(bar: impl T) {
1858+
buffer.puts(
1859+
*row_num - 1,
1860+
0,
1861+
&self.maybe_anonymized(line_start + line_pos),
1862+
Style::LineNumber,
1863+
);
1864+
buffer.puts(*row_num - 1, max_line_num_len + 1, "- ", Style::Removal);
1865+
buffer.puts(
1866+
*row_num - 1,
1867+
max_line_num_len + 3,
1868+
&normalize_whitespace(
1869+
&*file_lines
1870+
.file
1871+
.get_line(file_lines.lines[line_pos].line_index)
1872+
.unwrap(),
1873+
),
1874+
Style::NoStyle,
1875+
);
1876+
buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition);
1877+
} else if is_multiline {
1878+
match &highlight_parts[..] {
1879+
[SubstitutionHighlight { start: 0, end }] if *end == line.len() => {
1880+
buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition);
1881+
}
1882+
[] => {
1883+
draw_col_separator(buffer, *row_num, max_line_num_len + 1);
1884+
}
1885+
_ => {
1886+
buffer.puts(*row_num, max_line_num_len + 1, "~ ", Style::Addition);
1887+
}
18861888
}
1889+
} else {
1890+
draw_col_separator(buffer, *row_num, max_line_num_len + 1);
18871891
}
1888-
} else {
1889-
draw_col_separator(&mut buffer, row_num, max_line_num_len + 1);
1890-
}
18911892

1892-
// print the suggestion
1893-
buffer.append(row_num, &normalize_whitespace(line), Style::NoStyle);
1893+
// print the suggestion
1894+
buffer.append(*row_num, &normalize_whitespace(line), Style::NoStyle);
18941895

1895-
// Colorize addition/replacements with green.
1896-
for &SubstitutionHighlight { start, end } in highlight_parts {
1897-
// Account for tabs when highlighting (#87972).
1898-
let tabs: usize = line
1899-
.chars()
1900-
.take(start)
1901-
.map(|ch| match ch {
1902-
'\t' => 3,
1903-
_ => 0,
1904-
})
1905-
.sum();
1906-
buffer.set_style_range(
1907-
row_num,
1908-
max_line_num_len + 3 + start + tabs,
1909-
max_line_num_len + 3 + end + tabs,
1910-
Style::Addition,
1911-
true,
1912-
);
1896+
// Colorize addition/replacements with green.
1897+
for &SubstitutionHighlight { start, end } in highlight_parts {
1898+
// Account for tabs when highlighting (#87972).
1899+
let tabs: usize = line
1900+
.chars()
1901+
.take(start)
1902+
.map(|ch| match ch {
1903+
'\t' => 3,
1904+
_ => 0,
1905+
})
1906+
.sum();
1907+
buffer.set_style_range(
1908+
*row_num,
1909+
max_line_num_len + 3 + start + tabs,
1910+
max_line_num_len + 3 + end + tabs,
1911+
Style::Addition,
1912+
true,
1913+
);
1914+
}
1915+
*row_num += 1;
1916+
};
1917+
1918+
if highlight_parts.is_empty() {
1919+
unhighlighted_lines.push((line_pos, line));
1920+
continue;
19131921
}
1914-
row_num += 1;
1922+
1923+
match unhighlighted_lines.len() {
1924+
0 => (),
1925+
// Since we show first line, "..." line and last line,
1926+
// There is no reason to hide if there are 3 or less lines
1927+
// (because then we just replace a line with ... which is
1928+
// not helpful)
1929+
n if n <= 3 => unhighlighted_lines.drain(..).for_each(|(p, l)| {
1930+
print_line(p, l, &Vec::new(), &mut buffer, &mut row_num)
1931+
}),
1932+
_ => {
1933+
unhighlighted_lines
1934+
.drain(..1)
1935+
.next()
1936+
.map(|(p, l)| print_line(p, l, &Vec::new(), &mut buffer, &mut row_num));
1937+
buffer.puts(row_num, max_line_num_len - 1, "...", Style::LineNumber);
1938+
row_num += 1;
1939+
unhighlighted_lines
1940+
.pop()
1941+
.map(|(p, l)| print_line(p, l, &Vec::new(), &mut buffer, &mut row_num));
1942+
}
1943+
}
1944+
1945+
print_line(line_pos, line, highlight_parts, &mut buffer, &mut row_num)
19151946
}
19161947

19171948
// This offset and the ones below need to be signed to account for replacement code

0 commit comments

Comments
 (0)