Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Minor improvements to multilabel rendering #183

Merged
merged 1 commit into from
Mar 9, 2020
Merged

Minor improvements to multilabel rendering #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions codespan-reporting/examples/term.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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