From 004e6f9388aeb2755d9903d98f83d13ba7a9d172 Mon Sep 17 00:00:00 2001 From: Scott Schafer Date: Fri, 14 Jun 2024 12:34:37 -0600 Subject: [PATCH 1/2] test: Cleanup ann_multiline2 source --- tests/fixtures/no-color/ann_multiline2.svg | 4 ++-- tests/fixtures/no-color/ann_multiline2.toml | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/fixtures/no-color/ann_multiline2.svg b/tests/fixtures/no-color/ann_multiline2.svg index 18a9bf6..de5c6f0 100644 --- a/tests/fixtures/no-color/ann_multiline2.svg +++ b/tests/fixtures/no-color/ann_multiline2.svg @@ -22,11 +22,11 @@ | - 26 | This is an exampl + 26 | This is an example | ____________^ - 27 | | e of an edge case of an annotation overflowing + 27 | | of an edge case of an annotation overflowing | |_^ this should not be on separate lines diff --git a/tests/fixtures/no-color/ann_multiline2.toml b/tests/fixtures/no-color/ann_multiline2.toml index dd85332..afb3aa9 100644 --- a/tests/fixtures/no-color/ann_multiline2.toml +++ b/tests/fixtures/no-color/ann_multiline2.toml @@ -5,8 +5,8 @@ title = "spacing error found" [[message.snippets]] source = """ -This is an exampl -e of an edge case of an annotation overflowing +This is an example +of an edge case of an annotation overflowing to exactly one character on next line. """ line_start = 26 @@ -15,4 +15,4 @@ fold = false [[message.snippets.annotations]] label = "this should not be on separate lines" level = "Error" -range = [11, 18] +range = [11, 19] From c68600d669efa3c9a371067c770d711598b3422e Mon Sep 17 00:00:00 2001 From: Scott Schafer Date: Fri, 14 Jun 2024 16:46:53 -0600 Subject: [PATCH 2/2] fix: Improve annotating line endings --- src/renderer/display_list.rs | 87 +++- tests/fixtures/no-color/ann_multiline2.svg | 14 +- .../fixtures/no-color/fold_ann_multiline.svg | 14 +- tests/formatter.rs | 429 ++++++++++++++++++ 4 files changed, 511 insertions(+), 33 deletions(-) diff --git a/src/renderer/display_list.rs b/src/renderer/display_list.rs index d94a660..bc46f7f 100644 --- a/src/renderer/display_list.rs +++ b/src/renderer/display_list.rs @@ -524,6 +524,7 @@ pub(crate) enum DisplaySourceLine<'a> { Content { text: &'a str, range: (usize, usize), // meta information for annotation placement. + end_line: EndLine, }, /// An empty source line. Empty, @@ -658,7 +659,8 @@ impl<'a> CursorLines<'a> { } } -enum EndLine { +#[derive(Copy, Clone, Debug, PartialEq)] +pub(crate) enum EndLine { Eof = 0, Crlf = 1, Lf = 2, @@ -847,13 +849,20 @@ fn format_header<'a>( for item in body { if let DisplayLine::Source { - line: DisplaySourceLine::Content { text, range }, + line: + DisplaySourceLine::Content { + text, + range, + end_line, + }, lineno, .. } = item { - if main_range >= range.0 && main_range <= range.1 { - let char_column = text[0..(main_range - range.0)].chars().count(); + if main_range >= range.0 && main_range <= range.1 + *end_line as usize { + let char_column = text[0..(main_range - range.0).min(text.len())] + .chars() + .count(); col = char_column + 1; line_offset = lineno.unwrap_or(1); break; @@ -927,8 +936,18 @@ fn fold_body(body: Vec>) -> Vec> { let mut unhighlighed_lines = vec![]; for line in body { match &line { - DisplayLine::Source { annotations, .. } => { - if annotations.is_empty() { + DisplayLine::Source { + annotations, + inline_marks, + .. + } => { + if annotations.is_empty() + // A multiline start mark (`/`) needs be treated as an + // annotation or the line could get folded. + && inline_marks + .iter() + .all(|m| m.mark_type != DisplayMarkType::AnnotationStart) + { unhighlighed_lines.push(line); } else { if lines.is_empty() { @@ -1016,12 +1035,14 @@ fn format_body( for (idx, (line, end_line)) in CursorLines::new(snippet.source).enumerate() { let line_length: usize = line.len(); let line_range = (current_index, current_index + line_length); + let end_line_size = end_line as usize; body.push(DisplayLine::Source { lineno: Some(current_line), inline_marks: vec![], line: DisplaySourceLine::Content { text: line, range: line_range, + end_line, }, annotations: vec![], }); @@ -1045,7 +1066,7 @@ fn format_body( let line_start_index = line_range.0; let line_end_index = line_range.1; current_line += 1; - current_index += line_length + end_line as usize; + current_index += line_length + end_line_size; // It would be nice to use filter_drain here once it's stable. annotations.retain(|annotation| { @@ -1057,18 +1078,24 @@ fn format_body( }; let label_right = annotation.label.map_or(0, |label| label.len() + 1); match annotation.range { - Range { start, .. } if start > line_end_index => true, + // This handles if the annotation is on the next line. We add + // the `end_line_size` to account for annotating the line end. + Range { start, .. } if start > line_end_index + end_line_size => true, + // This handles the case where an annotation is contained + // within the current line including any line-end characters. Range { start, end } - if start >= line_start_index && end <= line_end_index - // Allow annotating eof or stripped eol - || start == line_end_index && end - start <= 1 => + if start >= line_start_index + // We add at least one to `line_end_index` to allow + // highlighting the end of a file + && end <= line_end_index + max(end_line_size, 1) => { if let DisplayLine::Source { ref mut annotations, .. } = body[body_idx] { - let annotation_start_col = line[0..(start - line_start_index)] + let annotation_start_col = line + [0..(start - line_start_index).min(line_length)] .chars() .map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0)) .sum::(); @@ -1101,11 +1128,16 @@ fn format_body( } false } + // This handles the case where a multiline annotation starts + // somewhere on the current line, including any line-end chars Range { start, end } if start >= line_start_index - && start <= line_end_index + // The annotation can start on a line ending + && start <= line_end_index + end_line_size.saturating_sub(1) && end > line_end_index => { + // Special case for multiline annotations that start at the + // beginning of a line, which requires a special mark (`/`) if start - line_start_index == 0 { if let DisplayLine::Source { ref mut inline_marks, @@ -1122,7 +1154,8 @@ fn format_body( .. } = body[body_idx] { - let annotation_start_col = line[0..(start - line_start_index)] + let annotation_start_col = line + [0..(start - line_start_index).min(line_length)] .chars() .map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0)) .sum::(); @@ -1147,7 +1180,11 @@ fn format_body( } true } - Range { start, end } if start < line_start_index && end > line_end_index => { + // This handles the case where a multiline annotation starts + // somewhere before this line and ends after it as well + Range { start, end } + if start < line_start_index && end > line_end_index + max(end_line_size, 1) => + { if let DisplayLine::Source { ref mut inline_marks, .. @@ -1160,10 +1197,14 @@ fn format_body( } true } + // This handles the case where a multiline annotation ends + // somewhere on the current line, including any line-end chars Range { start, end } if start < line_start_index && end >= line_start_index - && end <= line_end_index => + // We add at least one to `line_end_index` to allow + // highlighting the end of a file + && end <= line_end_index + max(end_line_size, 1) => { if let DisplayLine::Source { ref mut inline_marks, @@ -1175,13 +1216,21 @@ fn format_body( mark_type: DisplayMarkType::AnnotationThrough, annotation_type: DisplayAnnotationType::from(annotation.level), }); - let end_mark = line[0..(end - line_start_index)] + let end_mark = line[0..(end - line_start_index).min(line_length)] .chars() .map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0)) .sum::() .saturating_sub(1); - - let end_plus_one = end_mark + 1; + // If the annotation ends on a line-end character, we + // need to annotate one past the end of the line + let (end_mark, end_plus_one) = if end > line_end_index + // Special case for highlighting the end of a file + || (end == line_end_index + 1 && end_line_size == 0) + { + (end_mark + 1, end_mark + 2) + } else { + (end_mark, end_mark + 1) + }; span_left_margin = min(span_left_margin, end_mark); span_right_margin = max(span_right_margin, end_plus_one); diff --git a/tests/fixtures/no-color/ann_multiline2.svg b/tests/fixtures/no-color/ann_multiline2.svg index de5c6f0..49c2c4b 100644 --- a/tests/fixtures/no-color/ann_multiline2.svg +++ b/tests/fixtures/no-color/ann_multiline2.svg @@ -1,4 +1,4 @@ - + | - 26 | This is an example + 26 | This is an example - | ____________^ + | ^^^^^^^ this should not be on separate lines - 27 | | of an edge case of an annotation overflowing + 27 | of an edge case of an annotation overflowing - | |_^ this should not be on separate lines + 28 | to exactly one character on next line. - 28 | to exactly one character on next line. - - | + | diff --git a/tests/fixtures/no-color/fold_ann_multiline.svg b/tests/fixtures/no-color/fold_ann_multiline.svg index 0d2d67c..f82fe25 100644 --- a/tests/fixtures/no-color/fold_ann_multiline.svg +++ b/tests/fixtures/no-color/fold_ann_multiline.svg @@ -1,4 +1,4 @@ - + 52 | / for ann in annotations { - ... | + 53 | | match (ann.range.0, ann.range.1) { - 71 | | } + ... | - 72 | | } + 71 | | } - | |_____^ expected enum `std::option::Option`, found () + 72 | | } - | + | |_____^ expected enum `std::option::Option`, found () + + | diff --git a/tests/formatter.rs b/tests/formatter.rs index 5b746e1..fa3927c 100644 --- a/tests/formatter.rs +++ b/tests/formatter.rs @@ -303,3 +303,432 @@ LL | abc let renderer = Renderer::plain().anonymized_line_numbers(true); assert_data_eq!(renderer.render(input).to_string(), expected); } + +#[test] +fn issue_130() { + let input = Level::Error.title("dummy").snippet( + Snippet::source("foo\nbar\nbaz") + .origin("file/path") + .line_start(3) + .fold(true) + .annotation(Level::Error.span(4..11)), // bar\nbaz + ); + + let expected = str![[r#" +error: dummy + --> file/path:4:1 + | +4 | / bar +5 | | baz + | |___^ + | +"#]]; + let renderer = Renderer::plain(); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn unterminated_string_multiline() { + let source = "\ +a\" +// ... +"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .fold(true) + .annotation(Level::Error.span(0..10)), // 1..10 works + ); + let expected = str![[r#" +error + --> file/path:3:1 + | +3 | / a" +4 | | // ... + | |_______^ + | +"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn char_and_nl_annotate_char() { + let source = "a\r\nb"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .annotation(Level::Error.span(0..2)), // a\r + ); + let expected = str![[r#" +error + --> file/path:3:1 + | +3 | a + | ^ +4 | b + |"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn char_eol_annotate_char() { + let source = "a\r\nb"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .annotation(Level::Error.span(0..3)), // a\r\n + ); + let expected = str![[r#" +error + --> file/path:3:1 + | +3 | a + | ^ +4 | b + |"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn char_eol_annotate_char_double_width() { + let snippets = Level::Error.title("").snippet( + Snippet::source("こん\r\nにちは\r\n世界") + .origin("") + .annotation(Level::Error.span(3..8)), // ん\r\n + ); + + let expected = str![[r#" +error + --> :1:2 + | +1 | こん + | ^^ +2 | にちは +3 | 世界 + | +"#]]; + + let renderer = Renderer::plain(); + assert_data_eq!(renderer.render(snippets).to_string(), expected); +} + +#[test] +fn annotate_eol() { + let source = "a\r\nb"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .annotation(Level::Error.span(1..2)), // \r + ); + let expected = str![[r#" +error + --> file/path:3:2 + | +3 | a + | ^ +4 | b + |"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn annotate_eol2() { + let source = "a\r\nb"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .annotation(Level::Error.span(1..3)), // \r\n + ); + let expected = str![[r#" +error + --> file/path:3:2 + | +3 | a + | ^ +4 | b + |"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn annotate_eol3() { + let source = "a\r\nb"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .annotation(Level::Error.span(2..3)), // \n + ); + let expected = str![[r#" +error + --> file/path:3:2 + | +3 | a + | ^ +4 | b + |"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn annotate_eol4() { + let source = "a\r\nb"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .annotation(Level::Error.span(2..2)), // \n + ); + let expected = str![[r#" +error + --> file/path:3:2 + | +3 | a + | ^ +4 | b + |"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn annotate_eol_double_width() { + let snippets = Level::Error.title("").snippet( + Snippet::source("こん\r\nにちは\r\n世界") + .origin("") + .annotation(Level::Error.span(7..8)), // \n + ); + + let expected = str![[r#" +error + --> :1:3 + | +1 | こん + | ^ +2 | にちは +3 | 世界 + | +"#]]; + + let renderer = Renderer::plain(); + assert_data_eq!(renderer.render(snippets).to_string(), expected); +} + +#[test] +fn multiline_eol_start() { + let source = "a\r\nb"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .annotation(Level::Error.span(1..4)), // \r\nb + ); + let expected = str![[r#" +error + --> file/path:3:2 + | +3 | a + | __^ +4 | | b + | |_^ + |"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn multiline_eol_start2() { + let source = "a\r\nb"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .annotation(Level::Error.span(2..4)), // \nb + ); + let expected = str![[r#" +error + --> file/path:3:2 + | +3 | a + | __^ +4 | | b + | |_^ + |"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn multiline_eol_start3() { + let source = "a\nb"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .annotation(Level::Error.span(1..3)), // \nb + ); + let expected = str![[r#" +error + --> file/path:3:2 + | +3 | a + | __^ +4 | | b + | |_^ + |"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn multiline_eol_start_double_width() { + let snippets = Level::Error.title("").snippet( + Snippet::source("こん\r\nにちは\r\n世界") + .origin("") + .annotation(Level::Error.span(7..11)), // \r\nに + ); + + let expected = str![[r#" +error + --> :1:3 + | +1 | こん + | _____^ +2 | | にちは + | |__^ +3 | 世界 + | +"#]]; + + let renderer = Renderer::plain(); + assert_data_eq!(renderer.render(snippets).to_string(), expected); +} + +#[test] +fn multiline_eol_start_eol_end() { + let source = "a\nb\nc"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .annotation(Level::Error.span(1..4)), // \nb\n + ); + let expected = str![[r#" +error + --> file/path:3:2 + | +3 | a + | __^ +4 | | b + | |__^ +5 | c + | +"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn multiline_eol_start_eol_end2() { + let source = "a\r\nb\r\nc"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .annotation(Level::Error.span(2..5)), // \nb\r + ); + let expected = str![[r#" +error + --> file/path:3:2 + | +3 | a + | __^ +4 | | b + | |__^ +5 | c + | +"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn multiline_eol_start_eol_end3() { + let source = "a\r\nb\r\nc"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .annotation(Level::Error.span(2..6)), // \nb\r\n + ); + let expected = str![[r#" +error + --> file/path:3:2 + | +3 | a + | __^ +4 | | b + | |__^ +5 | c + | +"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn multiline_eol_start_eof_end() { + let source = "a\r\nb"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .annotation(Level::Error.span(1..5)), // \r\nb(EOF) + ); + let expected = str![[r#" +error + --> file/path:3:2 + | +3 | a + | __^ +4 | | b + | |__^ + | +"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +} + +#[test] +fn multiline_eol_start_eof_end_double_width() { + let source = "ん\r\nに"; + let input = Level::Error.title("").snippet( + Snippet::source(source) + .origin("file/path") + .line_start(3) + .annotation(Level::Error.span(3..9)), // \r\nに(EOF) + ); + let expected = str![[r#" +error + --> file/path:3:2 + | +3 | ん + | ___^ +4 | | に + | |___^ + | +"#]]; + let renderer = Renderer::plain().anonymized_line_numbers(false); + assert_data_eq!(renderer.render(input).to_string(), expected); +}