1
Fork 0

review comments

This commit is contained in:
Esteban Küber 2016-11-22 23:32:16 -08:00
parent eb53ca3aad
commit b7982bbbe0
7 changed files with 191 additions and 114 deletions

View file

@ -14,7 +14,7 @@ use syntax_pos::{COMMAND_LINE_SP, DUMMY_SP, FileMap, Span, MultiSpan, CharPos};
use {Level, CodeSuggestion, DiagnosticBuilder, SubDiagnostic, CodeMapper}; use {Level, CodeSuggestion, DiagnosticBuilder, SubDiagnostic, CodeMapper};
use RenderSpan::*; use RenderSpan::*;
use snippet::{Annotation, AnnotationType, Line, StyledString, Style}; use snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, StyledString, Style};
use styled_buffer::StyledBuffer; use styled_buffer::StyledBuffer;
use std::io::prelude::*; use std::io::prelude::*;
@ -181,12 +181,17 @@ impl EmitterWriter {
if is_minimized { if is_minimized {
ann.annotation_type = AnnotationType::Minimized; ann.annotation_type = AnnotationType::Minimized;
} else if lo.line != hi.line { } else if lo.line != hi.line {
ann.annotation_type = AnnotationType::Multiline { let ml = MultilineAnnotation {
depth: 1, depth: 1,
line_start: lo.line, line_start: lo.line,
line_end: hi.line, line_end: hi.line,
start_col: lo.col.0,
end_col: hi.col.0,
is_primary: span_label.is_primary,
label: span_label.label.clone(),
}; };
multiline_annotations.push((lo.file.clone(), ann.clone())); ann.annotation_type = AnnotationType::Multiline(ml.clone());
multiline_annotations.push((lo.file.clone(), ml));
}; };
if !ann.is_multiline() { if !ann.is_multiline() {
@ -200,65 +205,34 @@ impl EmitterWriter {
// Find overlapping multiline annotations, put them at different depths // Find overlapping multiline annotations, put them at different depths
multiline_annotations.sort_by(|a, b| { multiline_annotations.sort_by(|a, b| {
if let AnnotationType::Multiline { (a.1.line_start, a.1.line_end).cmp(&(b.1.line_start, b.1.line_end))
line_start: a_start,
line_end: a_end,
..
} = a.1.annotation_type {
if let AnnotationType::Multiline {
line_start: b_start,
line_end: b_end,
..
} = b.1.annotation_type {
(a_start, a_end).cmp(&(b_start, b_end))
} else {
panic!("tried to sort multiline annotations, but found `{:?}`", b)
}
} else {
panic!("tried to sort multiline annotations, but found `{:?}`", a)
}
}); });
for item in multiline_annotations.clone() { for item in multiline_annotations.clone() {
let ann = item.1; let ann = item.1;
if let AnnotationType::Multiline {line_start, line_end, ..} = ann.annotation_type {
for item in multiline_annotations.iter_mut() { for item in multiline_annotations.iter_mut() {
let ref mut a = item.1; let ref mut a = item.1;
if let AnnotationType::Multiline {
line_start: start,
line_end: end,
..
} = a.annotation_type {
// Move all other multiline annotations overlapping with this one // Move all other multiline annotations overlapping with this one
// one level to the right. // one level to the right.
if &ann != a && num_overlap(line_start, line_end, start, end, true) { if &ann != a &&
a.annotation_type.increase_depth(); num_overlap(ann.line_start, ann.line_end, a.line_start, a.line_end, true)
{
a.increase_depth();
} else { } else {
break; break;
} }
} else {
panic!("tried to find depth for multiline annotation, but found `{:?}`",
ann)
};
} }
} else {
panic!("tried to find depth for multiline annotation, but found `{:?}`", ann)
};
} }
let mut max_depth = 0; // max overlapping multiline spans let mut max_depth = 0; // max overlapping multiline spans
for (file, ann) in multiline_annotations { for (file, ann) in multiline_annotations {
if let AnnotationType::Multiline {line_start, line_end, depth} = ann.annotation_type { if ann.depth > max_depth {
if depth > max_depth { max_depth = ann.depth;
max_depth = depth;
} }
add_annotation_to_file(&mut output, file.clone(), line_start, ann.as_start()); add_annotation_to_file(&mut output, file.clone(), ann.line_start, ann.as_start());
for line in line_start + 1..line_end { for line in ann.line_start + 1..ann.line_end {
add_annotation_to_file(&mut output, file.clone(), line, ann.as_line()); add_annotation_to_file(&mut output, file.clone(), line, ann.as_line());
} }
add_annotation_to_file(&mut output, file, line_end, ann.as_end()); add_annotation_to_file(&mut output, file, ann.line_end, ann.as_end());
} else {
panic!("non-multiline annotation `{:?}` in `multiline_annotations`!", ann);
}
} }
for file_vec in output.iter_mut() { for file_vec in output.iter_mut() {
file_vec.multiline_depth = max_depth; file_vec.multiline_depth = max_depth;
@ -572,7 +546,7 @@ impl EmitterWriter {
// | | something about `foo` // | | something about `foo`
// | something about `fn foo()` // | something about `fn foo()`
annotations_position.sort_by(|a, b| { annotations_position.sort_by(|a, b| {
fn len(a: Annotation) -> usize { fn len(a: &Annotation) -> usize {
// Account for usize underflows // Account for usize underflows
if a.end_col > a.start_col { if a.end_col > a.start_col {
a.end_col - a.start_col a.end_col - a.start_col

View file

@ -41,6 +41,57 @@ pub struct Line {
pub annotations: Vec<Annotation>, pub annotations: Vec<Annotation>,
} }
#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
pub struct MultilineAnnotation {
pub depth: usize,
pub line_start: usize,
pub line_end: usize,
pub start_col: usize,
pub end_col: usize,
pub is_primary: bool,
pub label: Option<String>,
}
impl MultilineAnnotation {
pub fn increase_depth(&mut self) {
self.depth += 1;
}
pub fn as_start(&self) -> Annotation {
Annotation {
start_col: self.start_col,
end_col: self.start_col + 1,
is_primary: self.is_primary,
label: Some("starting here...".to_owned()),
annotation_type: AnnotationType::MultilineStart(self.depth)
}
}
pub fn as_end(&self) -> Annotation {
Annotation {
start_col: self.end_col - 1,
end_col: self.end_col,
is_primary: self.is_primary,
label: match self.label {
Some(ref label) => Some(format!("...ending here: {}", label)),
None => Some("...ending here".to_owned()),
},
annotation_type: AnnotationType::MultilineEnd(self.depth)
}
}
pub fn as_line(&self) -> Annotation {
Annotation {
start_col: 0,
end_col: 0,
is_primary: self.is_primary,
label: None,
annotation_type: AnnotationType::MultilineLine(self.depth)
}
}
}
#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)] #[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
pub enum AnnotationType { pub enum AnnotationType {
/// Annotation under a single line of code /// Annotation under a single line of code
@ -50,11 +101,7 @@ pub enum AnnotationType {
Minimized, Minimized,
/// Annotation enclosing the first and last character of a multiline span /// Annotation enclosing the first and last character of a multiline span
Multiline { Multiline(MultilineAnnotation),
depth: usize,
line_start: usize,
line_end: usize,
},
// The Multiline type above is replaced with the following three in order // The Multiline type above is replaced with the following three in order
// to reuse the current label drawing code. // to reuse the current label drawing code.
@ -74,24 +121,6 @@ pub enum AnnotationType {
MultilineLine(usize), MultilineLine(usize),
} }
impl AnnotationType {
pub fn depth(&self) -> usize {
match self {
&AnnotationType::Multiline {depth, ..} |
&AnnotationType::MultilineStart(depth) |
&AnnotationType::MultilineLine(depth) |
&AnnotationType::MultilineEnd(depth) => depth,
_ => 0,
}
}
pub fn increase_depth(&mut self) {
if let AnnotationType::Multiline {ref mut depth, ..} = *self {
*depth += 1;
}
}
}
#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)] #[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
pub struct Annotation { pub struct Annotation {
/// Start column, 0-based indexing -- counting *characters*, not /// Start column, 0-based indexing -- counting *characters*, not
@ -124,7 +153,7 @@ impl Annotation {
pub fn is_multiline(&self) -> bool { pub fn is_multiline(&self) -> bool {
match self.annotation_type { match self.annotation_type {
AnnotationType::Multiline {..} | AnnotationType::Multiline(_) |
AnnotationType::MultilineStart(_) | AnnotationType::MultilineStart(_) |
AnnotationType::MultilineLine(_) | AnnotationType::MultilineLine(_) |
AnnotationType::MultilineEnd(_) => true, AnnotationType::MultilineEnd(_) => true,
@ -132,31 +161,6 @@ impl Annotation {
} }
} }
pub fn as_start(&self) -> Annotation {
let mut a = self.clone();
a.annotation_type = AnnotationType::MultilineStart(self.annotation_type.depth());
a.end_col = a.start_col + 1;
a.label = Some("starting here...".to_owned());
a
}
pub fn as_end(&self) -> Annotation {
let mut a = self.clone();
a.annotation_type = AnnotationType::MultilineEnd(self.annotation_type.depth());
a.start_col = a.end_col - 1;
a.label = match a.label {
Some(l) => Some(format!("...ending here: {}", l)),
None => Some("..ending here".to_owned()),
};
a
}
pub fn as_line(&self) -> Annotation {
let mut a = self.clone();
a.annotation_type = AnnotationType::MultilineLine(self.annotation_type.depth());
a.label = None;
a
}
} }
#[derive(Debug)] #[derive(Debug)]

View file

@ -444,3 +444,103 @@ error: foo
"#); "#);
} }
#[test]
fn non_overlaping() {
test_harness(r#"
fn foo() {
X0 Y0 Z0
X1 Y1 Z1
X2 Y2 Z2
X3 Y3 Z3
}
"#,
vec![
SpanLabel {
start: Position {
string: "Y0",
count: 1,
},
end: Position {
string: "X1",
count: 1,
},
label: "`X` is a good letter",
},
SpanLabel {
start: Position {
string: "Y2",
count: 1,
},
end: Position {
string: "Z3",
count: 1,
},
label: "`Y` is a good letter too",
},
],
r#"
error: foo
--> test.rs:3:6
|
3 | X0 Y0 Z0
| ______^ starting here...
4 | | X1 Y1 Z1
| |____^ ...ending here: `X` is a good letter
5 | X2 Y2 Z2
| ______- starting here...
6 | | X3 Y3 Z3
| |__________- ...ending here: `Y` is a good letter too
"#);
}
#[test]
fn overlaping_start_and_end() {
test_harness(r#"
fn foo() {
X0 Y0 Z0
X1 Y1 Z1
X2 Y2 Z2
X3 Y3 Z3
}
"#,
vec![
SpanLabel {
start: Position {
string: "Y0",
count: 1,
},
end: Position {
string: "X1",
count: 1,
},
label: "`X` is a good letter",
},
SpanLabel {
start: Position {
string: "Z1",
count: 1,
},
end: Position {
string: "Z3",
count: 1,
},
label: "`Y` is a good letter too",
},
],
r#"
error: foo
--> test.rs:3:6
|
3 | X0 Y0 Z0
| ______^ starting here...
4 | | X1 Y1 Z1
| |____^____- starting here...
| ||____|
| | ...ending here: `X` is a good letter
5 | | X2 Y2 Z2
6 | | X3 Y3 Z3
| |___________- ...ending here: `Y` is a good letter too
"#);
}

View file

@ -8,7 +8,7 @@ error[E0569]: requires an `unsafe impl` declaration due to `#[may_dangle]` attri
35 | | // (unsafe to access self.1 due to #[may_dangle] on A) 35 | | // (unsafe to access self.1 due to #[may_dangle] on A)
36 | | fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); } 36 | | fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); }
37 | | } 37 | | }
| |_^ ..ending here | |_^ ...ending here
error[E0569]: requires an `unsafe impl` declaration due to `#[may_dangle]` attribute error[E0569]: requires an `unsafe impl` declaration due to `#[may_dangle]` attribute
--> $DIR/dropck-eyepatch-implies-unsafe-impl.rs:38:1 --> $DIR/dropck-eyepatch-implies-unsafe-impl.rs:38:1
@ -20,7 +20,7 @@ error[E0569]: requires an `unsafe impl` declaration due to `#[may_dangle]` attri
41 | | // (unsafe to access self.1 due to #[may_dangle] on 'a) 41 | | // (unsafe to access self.1 due to #[may_dangle] on 'a)
42 | | fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); } 42 | | fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); }
43 | | } 43 | | }
| |_^ ..ending here | |_^ ...ending here
error: aborting due to 2 previous errors error: aborting due to 2 previous errors

View file

@ -19,7 +19,7 @@ help: consider using an explicit lifetime parameter as shown: fn from_str(path:
| _____^ starting here... | _____^ starting here...
26 | | Ok(Foo { field: path }) 26 | | Ok(Foo { field: path })
27 | | } 27 | | }
| |_____^ ..ending here | |_____^ ...ending here
error: aborting due to 2 previous errors error: aborting due to 2 previous errors

View file

@ -20,7 +20,7 @@ fn main() {
let x = 1; let x = 1;
let y = 2; let y = 2;
let z = 3; let z = 3;
foo(1 + foo(1 as u32 +
bar(x, bar(x,

View file

@ -1,20 +1,19 @@
error[E0277]: the trait bound `{integer}: std::ops::Add<()>` is not satisfied error[E0277]: the trait bound `u32: std::ops::Add<()>` is not satisfied
--> $DIR/multiline-span-simple.rs:23:9 --> $DIR/multiline-span-simple.rs:23:9
| |
23 | foo(1 + 23 | foo(1 as u32 +
| _________^ starting here... | _________^ starting here...
24 | | 24 | |
25 | | bar(x, 25 | | bar(x,
26 | | 26 | |
27 | | y), 27 | | y),
| |______________^ ...ending here: the trait `std::ops::Add<()>` is not implemented for `{integer}` | |______________^ ...ending here: the trait `std::ops::Add<()>` is not implemented for `u32`
| |
= help: the following implementations were found: = help: the following implementations were found:
= help: <u32 as std::ops::Add> = help: <u32 as std::ops::Add>
= help: <&'a u32 as std::ops::Add<u32>> = help: <&'a u32 as std::ops::Add<u32>>
= help: <u32 as std::ops::Add<&'a u32>> = help: <u32 as std::ops::Add<&'a u32>>
= help: <&'b u32 as std::ops::Add<&'a u32>> = help: <&'b u32 as std::ops::Add<&'a u32>>
= help: and 90 others
error: aborting due to previous error error: aborting due to previous error