Skip to content

Commit

Permalink
Minor improvements to multilabel rendering
Browse files Browse the repository at this point in the history 
Working towards #100, this slightly cleans up the rendering of diagnostics with multiple labels.
  • Loading branch information
@brendanzab
brendanzab committed Mar 9, 2020
1 parent b54cae4 commit 07f0da6
Show file tree
Hide file tree
Showing 7 changed files with 175 additions and 108 deletions.
13 changes: 8 additions & 5 deletions codespan-reporting/examples/term.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ fn main() {
_ 0 => "Buzz"
_ _ => num
fizz₂ : Nat → String
fizz₂ num =
case (mod num 5) (mod num 3) of
0 0 => "FizzBuzz"
Expand Down Expand Up @@ -132,15 +133,17 @@ fn main() {
.with_message("`case` clauses have incompatible types")
.with_code("E0308")
.with_labels(vec![
Label::primary(file_id3, 303..306).with_message("expected `String`, found `Nat`"),
Label::secondary(file_id3, 186..306)
Label::primary(file_id3, 328..331).with_message("expected `String`, found `Nat`"),
Label::secondary(file_id3, 211..331)
.with_message("`case` clauses have incompatible types"),
Label::secondary(file_id3, 233..243)
Label::secondary(file_id3, 258..268)
.with_message("this is found to be of type `String`"),
Label::secondary(file_id3, 259..265)
Label::secondary(file_id3, 284..290)
.with_message("this is found to be of type `String`"),
Label::secondary(file_id3, 281..287)
Label::secondary(file_id3, 306..312)
.with_message("this is found to be of type `String`"),
Label::secondary(file_id3, 186..192)
.with_message("expected type `String` found here"),
])
.with_notes(vec![unindent::unindent(
"
Expand Down
168 changes: 116 additions & 52 deletions codespan-reporting/src/term/views.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,34 +38,49 @@ where
// TODO: Make this data structure external, to allow for allocation reuse
let mut file_ids_to_labels = Vec::new();
let mut outer_padding = 0;

// Group marks by file
for label in &self.diagnostic.labels {
let start_line_index = files
.line_index(label.file_id, label.range.start)
.expect("start_line_index");
let end_line_index = files
.line_index(label.file_id, label.range.end)
.expect("end_line_index");
let end_line = files.line(label.file_id, end_line_index).expect("end_line");
// The label spans over multiple lines if if the line indices of the
// start and end differ.
let is_multiline = start_line_index != end_line_index;
// Update the outer padding based on the last lin number
outer_padding = std::cmp::max(outer_padding, count_digits(end_line.number));

// TODO: Group contiguous line index ranges using some sort of interval set algorithm.
// TODO: Flatten mark groups to overlapping underlines that can be easily rendered.
// TODO: If start line and end line are too far apart, we should add a source break.
match file_ids_to_labels
.iter_mut()
.find(|(file_id, _)| label.file_id == *file_id)
.find(|(file_id, _, _)| label.file_id == *file_id)
{
None => file_ids_to_labels.push((label.file_id, vec![label])),
Some((_, labels)) => labels.push(label),
None => file_ids_to_labels.push((label.file_id, is_multiline, vec![label])),
Some((_, seen_multiline, labels)) => {
// Keep track of if we've sen a multiline label. This helps
// us to figure out the inner padding of the source snippet.
// TODO: This will need to be more complicated once we allow multiline labels to overlap.
*seen_multiline |= is_multiline;
// Ensure that the vector of labels is sorted
// lexicographically by the range of source code they cover.
// This should make our job easier later on.
match labels.binary_search_by(|other| {
// `Range<usize>` doesn't implement `Ord`, so convert to `(usize, usize)`
// to piggyback off its lexicographic sorting implementation.
(other.range.start, other.range.end)
.cmp(&(label.range.start, label.range.end))
}) {
Ok(i) | Err(i) => labels.insert(i, label),
}
}
}
}

// Sort labels lexicographically by the range of source code they cover.
for (_, labels) in file_ids_to_labels.iter_mut() {
// `Range<usize>` doesn't implement `Ord`, so convert to `(usize, usize)`
// to piggyback off its lexicographic sorting implementation.
labels.sort_by_key(|label| (label.range.start, label.range.end));
}

// Header and message
//
// ```text
Expand All @@ -91,53 +106,58 @@ where
// │ ^^ expected `Int` but found `String`
// │
// ```
for (file_id, labels) in &file_ids_to_labels {
for (file_id, seen_multiline, labels) in &file_ids_to_labels {
let source = files.source(*file_id).expect("source");
let mut labels = labels
.iter()
.map(|label| {
let start_line_index = files
.line_index(label.file_id, label.range.start)
.expect("start_line_index");
let start_line = files
.line(label.file_id, start_line_index)
.expect("start_line");
let end_line_index = files
.line_index(label.file_id, label.range.end)
.expect("end_line_index");
let end_line = files.line(label.file_id, end_line_index).expect("end_line");
(
label,
(start_line_index, start_line),
(end_line_index, end_line),
)
})
.peekable();

// Top left border and locus.
//
// ```text
// ┌── test:2:9 ───
// ```
if let Some((label, (_, start_line), _)) = labels.peek() {
let start_source = &source.as_ref()[start_line.range.clone()];
renderer.render_source_start(
outer_padding,
&Locus {
origin: files.origin(*file_id).expect("origin").to_string(),
line_number: start_line.number,
column_number: start_line.column_number(start_source, label.range.start),
},
)?;
renderer.render_source_empty(outer_padding, &[])?;
}

for (i, label) in labels.iter().enumerate() {
while let Some((label, (start_line_index, start_line), (end_line_index, end_line))) =
labels.next()
{
let severity = match label.style {
LabelStyle::Primary => Some(self.diagnostic.severity),
LabelStyle::Secondary => None,
};

let start_line_index = files
.line_index(label.file_id, label.range.start)
.expect("start_line_index");
let start_line = files
.line(label.file_id, start_line_index)
.expect("start_line");
let start_source = &source.as_ref()[start_line.range.clone()];
let end_line_index = files
.line_index(label.file_id, label.range.end)
.expect("end_line_index");
let end_line = files.line(label.file_id, end_line_index).expect("end_line");
let end_source = &source.as_ref()[end_line.range.clone()];

if i == 0 {
// Top left border and locus.
//
// ```text
// ┌── test:2:9 ───
// ```
renderer.render_source_start(
outer_padding,
&Locus {
origin: files.origin(*file_id).expect("origin").to_string(),
line_number: start_line.number,
column_number: start_line
.column_number(start_source, label.range.start),
},
)?;
renderer.render_source_empty(outer_padding, &[])?;
} else {
// Source break.
//
// ```text
// ·
// ```
renderer.render_source_break(outer_padding, &[])?;
};

// Attempt to split off the last line.
if start_line_index == end_line_index {
// Single line
Expand All @@ -149,14 +169,18 @@ where
let mark_start = label.range.start - start_line.range.start;
let mark_end = label.range.end - start_line.range.start;

let mark = || (severity, Mark::Single(mark_start..mark_end, &label.message));
let multi_marks = [None, Some(mark())];
let single_marks = [Some(mark())];

renderer.render_source_line(
outer_padding,
start_line.number,
start_source.as_ref(),
&[Some((
severity,
Mark::Single(mark_start..mark_end, &label.message),
))],
match seen_multiline {
true => &multi_marks,
false => &single_marks,
},
)?;
} else {
// Multiple lines
Expand Down Expand Up @@ -240,6 +264,46 @@ where
))],
)?;
}

if let Some((_, (next_start_line_index, _), _)) = labels.peek() {
match next_start_line_index.checked_sub(end_line_index) {
// Same line
Some(0) => {
// TODO: Accumulate marks!
renderer.render_source_break(outer_padding, &[])?;
}
// Consecutive lines
Some(1) => {}
// Only one line between us and the next label
Some(2) => {
// Write a source line
let next_line = files
.line(label.file_id, end_line_index + 1)
.expect("next_line");
let next_source = &source.as_ref()[next_line.range.clone()];
renderer.render_source_line(
outer_padding,
next_line.number,
next_source,
match seen_multiline {
true => &[None],
false => &[],
},
)?;
}
// Either:
// - one line between us and the next label
// - labels are out of order
Some(_) | None => {
// Source break
//
// ```text
// ·
// ```
renderer.render_source_break(outer_padding, &[])?;
}
}
}
}
renderer.render_source_empty(outer_padding, &[])?;
}
Expand Down
43 changes: 21 additions & 22 deletions codespan-reporting/tests/snapshots/term__fizz_buzz__rich_color.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ expression: TEST_DATA.emit_color(&config)

{fg:Blue}┌──{/} FizzBuzz.fun:3:15 {fg:Blue}───{/}
{fg:Blue}│{/}
{fg:Blue}3{/} {fg:Blue}│{/} fizz₁ : Nat → String
{fg:Blue}│{/} {fg:Blue}------ expected type `String` found here{/}
{fg:Blue}·{/}
{fg:Blue}3{/} {fg:Blue}│{/} fizz₁ : Nat → String
{fg:Blue}│{/} {fg:Blue}------ expected type `String` found here{/}
{fg:Blue}4{/} {fg:Blue}│{/} fizz₁ num = case (mod num 5) (mod num 3) of
{fg:Blue}│{/} {fg:Blue}╭{/}{fg:Blue}─────────────'{/}
{fg:Blue}5{/} {fg:Blue}│{/} {fg:Blue}│{/} 0 0 => "FizzBuzz"
Expand All @@ -17,34 +16,34 @@ expression: TEST_DATA.emit_color(&config)
{fg:Blue}8{/} {fg:Blue}│{/} {fg:Blue}│{/} _ _ => num
{fg:Blue}│{/} {fg:Blue}╰{/}{fg:Blue}──────────────' `case` clauses have incompatible types{/}
{fg:Blue}·{/}
{fg:Blue}8{/} {fg:Blue}│{/} _ _ => num
{fg:Blue}│{/} {fg:Red}^^^ expected `String`, found `Nat`{/}
{fg:Blue}8{/} {fg:Blue}│{/} _ _ => num
{fg:Blue}│{/} {fg:Red}^^^ expected `String`, found `Nat`{/}
{fg:Blue}│{/}
{fg:Blue}={/} expected type `String`
found type `Nat`

{fg:Red bold bright}error[E0308]{bold bright}: `case` clauses have incompatible types{/}

{fg:Blue}┌──{/} FizzBuzz.fun:11:5 {fg:Blue}───{/}
{fg:Blue}┌──{/} FizzBuzz.fun:10:15 {fg:Blue}───{/}
{fg:Blue}│{/}
{fg:Blue}11{/} {fg:Blue}│{/} {fg:Blue}╭{/} case (mod num 5) (mod num 3) of
{fg:Blue}12{/} {fg:Blue}│{/} {fg:Blue}│{/} 0 0 => "FizzBuzz"
{fg:Blue}13{/} {fg:Blue}│{/} {fg:Blue}│{/} 0 _ => "Fizz"
{fg:Blue}14{/} {fg:Blue}│{/} {fg:Blue}│{/} _ 0 => "Buzz"
{fg:Blue}15{/} {fg:Blue}│{/} {fg:Blue}│{/} _ _ => num
{fg:Blue}10{/} {fg:Blue}│{/} fizz₂ : Nat → String
{fg:Blue}│{/} {fg:Blue}------ expected type `String` found here{/}
{fg:Blue}11{/} {fg:Blue}│{/} fizz₂ num =
{fg:Blue}12{/} {fg:Blue}│{/} {fg:Blue}╭{/} case (mod num 5) (mod num 3) of
{fg:Blue}13{/} {fg:Blue}│{/} {fg:Blue}│{/} 0 0 => "FizzBuzz"
{fg:Blue}14{/} {fg:Blue}│{/} {fg:Blue}│{/} 0 _ => "Fizz"
{fg:Blue}15{/} {fg:Blue}│{/} {fg:Blue}│{/} _ 0 => "Buzz"
{fg:Blue}16{/} {fg:Blue}│{/} {fg:Blue}│{/} _ _ => num
{fg:Blue}│{/} {fg:Blue}╰{/}{fg:Blue}──────────────────' `case` clauses have incompatible types{/}
{fg:Blue}·{/}
{fg:Blue}12{/} {fg:Blue}│{/} 0 0 => "FizzBuzz"
{fg:Blue}│{/} {fg:Blue}---------- this is found to be of type `String`{/}
{fg:Blue}·{/}
{fg:Blue}13{/} {fg:Blue}│{/} 0 _ => "Fizz"
{fg:Blue}│{/} {fg:Blue}------ this is found to be of type `String`{/}
{fg:Blue}·{/}
{fg:Blue}14{/} {fg:Blue}│{/} _ 0 => "Buzz"
{fg:Blue}│{/} {fg:Blue}------ this is found to be of type `String`{/}
{fg:Blue}·{/}
{fg:Blue}15{/} {fg:Blue}│{/} _ _ => num
{fg:Blue}│{/} {fg:Red}^^^ expected `String`, found `Nat`{/}
{fg:Blue}13{/} {fg:Blue}│{/} 0 0 => "FizzBuzz"
{fg:Blue}│{/} {fg:Blue}---------- this is found to be of type `String`{/}
{fg:Blue}14{/} {fg:Blue}│{/} 0 _ => "Fizz"
{fg:Blue}│{/} {fg:Blue}------ this is found to be of type `String`{/}
{fg:Blue}15{/} {fg:Blue}│{/} _ 0 => "Buzz"
{fg:Blue}│{/} {fg:Blue}------ this is found to be of type `String`{/}
{fg:Blue}16{/} {fg:Blue}│{/} _ _ => num
{fg:Blue}│{/} {fg:Red}^^^ expected `String`, found `Nat`{/}
{fg:Blue}│{/}
{fg:Blue}={/} expected type `String`
found type `Nat`
Expand Down
43 changes: 21 additions & 22 deletions codespan-reporting/tests/snapshots/term__fizz_buzz__rich_no_color.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ error[E0308]: `case` clauses have incompatible types

┌── FizzBuzz.fun:3:15 ───
│
3 │ fizz₁ : Nat → String
│ ------ expected type `String` found here
·
3 │ fizz₁ : Nat → String
│ ------ expected type `String` found here
4 │ fizz₁ num = case (mod num 5) (mod num 3) of
│ ╭─────────────'
5 │ │ 0 0 => "FizzBuzz"
Expand All @@ -17,34 +16,34 @@ error[E0308]: `case` clauses have incompatible types
8 │ │ _ _ => num
│ ╰──────────────' `case` clauses have incompatible types
·
8 │ _ _ => num
│ ^^^ expected `String`, found `Nat`
8 │ _ _ => num
│ ^^^ expected `String`, found `Nat`
│
= expected type `String`
found type `Nat`

error[E0308]: `case` clauses have incompatible types

┌── FizzBuzz.fun:11:5 ───
┌── FizzBuzz.fun:10:15 ───
│
11 │ ╭ case (mod num 5) (mod num 3) of
12 │ │ 0 0 => "FizzBuzz"
13 │ │ 0 _ => "Fizz"
14 │ │ _ 0 => "Buzz"
15 │ │ _ _ => num
10 │ fizz₂ : Nat → String
│ ------ expected type `String` found here
11 │ fizz₂ num =
12 │ ╭ case (mod num 5) (mod num 3) of
13 │ │ 0 0 => "FizzBuzz"
14 │ │ 0 _ => "Fizz"
15 │ │ _ 0 => "Buzz"
16 │ │ _ _ => num
│ ╰──────────────────' `case` clauses have incompatible types
·
12 │ 0 0 => "FizzBuzz"
│ ---------- this is found to be of type `String`
·
13 │ 0 _ => "Fizz"
│ ------ this is found to be of type `String`
·
14 │ _ 0 => "Buzz"
│ ------ this is found to be of type `String`
·
15 │ _ _ => num
│ ^^^ expected `String`, found `Nat`
13 │ 0 0 => "FizzBuzz"
│ ---------- this is found to be of type `String`
14 │ 0 _ => "Fizz"
│ ------ this is found to be of type `String`
15 │ _ 0 => "Buzz"
│ ------ this is found to be of type `String`
16 │ _ _ => num
│ ^^^ expected `String`, found `Nat`
│
= expected type `String`
found type `Nat`
Expand Down
2 changes: 1 addition & 1 deletion codespan-reporting/tests/snapshots/term__fizz_buzz__short_color.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ source: codespan-reporting/tests/term.rs
expression: TEST_DATA.emit_color(&config)
---
FizzBuzz.fun:8:12: {fg:Red bold bright}error[E0308]{bold bright}: `case` clauses have incompatible types{/}
FizzBuzz.fun:15:16: {fg:Red bold bright}error[E0308]{bold bright}: `case` clauses have incompatible types{/}
FizzBuzz.fun:16:16: {fg:Red bold bright}error[E0308]{bold bright}: `case` clauses have incompatible types{/}

Loading

0 comments on commit 07f0da6

Please sign in to comment.