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.

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

no excessive highlighting #263

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
50 changes: 35 additions & 15 deletions codespan-reporting/src/term/renderer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,20 @@ impl<'writer, 'config> Renderer<'writer, 'config> {
// FIXME: Use the number of trimmed placeholders when rendering single line carets
let source = source.trim_end_matches(['\n', '\r', '\0'].as_ref());

// get the byte index of the first non-whitespace character
let text_start = source.find(|c| !char::is_whitespace(c)).unwrap_or(0);

// get the byte index of the first trailing whitespace character
let text_end = source
.char_indices()
.rev()
.take_while(|(_, c)| c.is_whitespace())
.last()
.map(|(index, _)| index)
.unwrap_or_else(|| source.len());
Comment on lines +246 to +252
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also be rewritten using str::rfind, but because we have to find the first trailing non-whitespace character, we would have to find the char lengt of the character to adjust, which makes it not much clearer than the curren solution.

Suggested change
let text_end = source
.char_indices()
.rev()
.take_while(|(_, c)| c.is_whitespace())
.last()
.map(|(index, _)| index)
.unwrap_or_else(|| source.len());
let text_end = source
.rfind(|c| !char::is_whitespace(c))
.map(|index|
index + source[index..]
.chars()
.next()
.map_or(0, |c| c.len_utf8())
)
.unwrap_or_else(|| source.len());


let text_range = text_start..text_end;

// Write source line
//
// ```text
Expand Down Expand Up @@ -274,37 +288,43 @@ impl<'writer, 'config> Renderer<'writer, 'config> {

// Write source text
write!(self, " ")?;
let mut in_primary = false;
let mut highlighted = false;

// iterate over all characters of source code
for (metrics, ch) in self.char_metrics(source.char_indices()) {
let column_range = metrics.byte_index..(metrics.byte_index + ch.len_utf8());

// Check if we are overlapping a primary label
let is_primary = single_labels.iter().any(|(ls, range, _)| {
*ls == LabelStyle::Primary && is_overlapping(range, &column_range)
}) || multi_labels.iter().any(|(_, ls, label)| {
/*
A character should be highlighted only if it is part of a primary single label which does not span the whole line
A label already spans the whole line if it does not cover leading/trailing whitespace.
*/
let should_highlight = single_labels.iter().any(|(ls, range, _)| {
*ls == LabelStyle::Primary
&& match label {
MultiLabel::Top(start) => column_range.start >= *start,
MultiLabel::Left => true,
MultiLabel::Bottom(start, _) => column_range.end <= *start,
}
// is this at the current position
&& is_overlapping(range, &column_range)
// is this not a whole line label
&& !(
range.start <= text_range.start
&& range.end >= text_range.end
)
});

// Set the source color if we are in a primary label
if is_primary && !in_primary {
if should_highlight && !highlighted {
self.set_color(self.styles().label(severity, LabelStyle::Primary))?;
in_primary = true;
} else if !is_primary && in_primary {
highlighted = true;
} else if !should_highlight && highlighted {
self.reset()?;
in_primary = false;
highlighted = false;
}

// actually write the character
match ch {
'\t' => (0..metrics.unicode_width).try_for_each(|_| write!(self, " "))?,
_ => write!(self, "{}", ch)?,
}
}
if in_primary {
if highlighted {
self.reset()?;
}
writeln!(self)?;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
source: codespan-reporting/tests/term.rs
expression: TEST_DATA.emit_color(&config)
---
{fg:Green bold bright}note{bold bright}: not highlighted{/}
{fg:Blue}┌─{/} test.txt:1:1
{fg:Blue}│{/}
{fg:Blue}1{/} {fg:Blue}│{/} This is some text.
{fg:Blue}│{/} {fg:Green}^^^^^^^^^^^^^^^^^^{/}
{fg:Blue}2{/} {fg:Blue}│{/} This is some indented text.
{fg:Blue}│{/} {fg:Green}^^^^^^^^^^^^^^^^^^^^^^^^^^^{/}
{fg:Blue}3{/} {fg:Blue}│{/} This is some indented text with trailing whitespace.
{fg:Blue}│{/} {fg:Green}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^{/}

{fg:Green bold bright}note{bold bright}: also not highlighted{/}
{fg:Blue}┌─{/} test.txt:2:1
{fg:Blue}│{/}
{fg:Blue}2{/} {fg:Blue}│{/} This is some indented text.
{fg:Blue}│{/} {fg:Green}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^{/}
{fg:Blue}3{/} {fg:Blue}│{/} This is some indented text with trailing whitespace.
{fg:Blue}│{/} {fg:Green}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^{/}

{fg:Green bold bright}note{bold bright}: highlighted{/}
{fg:Blue}┌─{/} test.txt:1:1
{fg:Blue}│{/}
{fg:Blue}1{/} {fg:Blue}│{/} {fg:Green}This is some text{/}.
{fg:Blue}│{/} {fg:Green}^^^^^^^^^^^^^^^^^{/}
{fg:Blue}2{/} {fg:Blue}│{/} T{fg:Green}his is some indented text.{/}
{fg:Blue}│{/} {fg:Green}^^^^^^^^^^^^^^^^^^^^^^^^^^{/}
{fg:Blue}3{/} {fg:Blue}│{/} {fg:Green}This{/} is some indented text with trailing whitespace.
{fg:Blue}│{/} {fg:Green}^^^^{/}
{fg:Blue}4{/} {fg:Blue}│{/} {fg:Green}╭{/} This is some multiline text.
{fg:Blue}5{/} {fg:Blue}│{/} {fg:Green}│{/} This is some more multiline text.
{fg:Blue}│{/} {fg:Green}╰{/}{fg:Green}─────────────────────────────────^{/}


Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ expression: TEST_DATA.emit_color(&config)
{fg:Blue}│{/} {fg:Blue}│{/} {fg:Blue}---------------------------------------------{/} {fg:Blue}this is found to be of type `Result<ByteIndex, LineIndexOutOfBoundsError>`{/}
{fg:Blue}3{/} {fg:Blue}│{/} {fg:Blue}│{/} Ordering::Equal => Ok(self.source_span().end()),
{fg:Blue}│{/} {fg:Blue}│{/} {fg:Blue}----------------------------{/} {fg:Blue}this is found to be of type `Result<ByteIndex, LineIndexOutOfBoundsError>`{/}
{fg:Blue}4{/} {fg:Blue}│{/} {fg:Blue}│{/} Ordering::Greater => {fg:Red}LineIndexOutOfBoundsError {{/}
{fg:Blue}4{/} {fg:Blue}│{/} {fg:Blue}│{/} Ordering::Greater => LineIndexOutOfBoundsError {
{fg:Blue}│{/} {fg:Red}╭{/}{fg:Red}─{/}{fg:Blue}│{/}{fg:Red}──────────────────────────────────^{/}
{fg:Blue}5{/} {fg:Blue}│{/} {fg:Red}│{/} {fg:Blue}│{/} {fg:Red} given: line_index,{/}
{fg:Blue}6{/} {fg:Blue}│{/} {fg:Red}│{/} {fg:Blue}│{/} {fg:Red} max: self.last_line_index(),{/}
{fg:Blue}7{/} {fg:Blue}│{/} {fg:Red}│{/} {fg:Blue}│{/} {fg:Red} }{/},
{fg:Blue}5{/} {fg:Blue}│{/} {fg:Red}│{/} {fg:Blue}│{/} given: line_index,
{fg:Blue}6{/} {fg:Blue}│{/} {fg:Red}│{/} {fg:Blue}│{/} max: self.last_line_index(),
{fg:Blue}7{/} {fg:Blue}│{/} {fg:Red}│{/} {fg:Blue}│{/} },
{fg:Blue}│{/} {fg:Red}╰{/}{fg:Red}─{/}{fg:Blue}│{/}{fg:Red}─────────────^ expected enum `Result`, found struct `LineIndexOutOfBoundsError`{/}
{fg:Blue}8{/} {fg:Blue}│{/} {fg:Blue}│{/} }
{fg:Blue}│{/} {fg:Blue}╰{/}{fg:Blue}─────────' `match` arms have incompatible types{/}
Expand Down
48 changes: 48 additions & 0 deletions codespan-reporting/tests/term.rs
Original file line number Diff line number Diff line change
Expand Up @@ -917,6 +917,7 @@ mod unicode_spans {
}

mod position_indicator {

use super::*;

lazy_static::lazy_static! {
Expand Down Expand Up @@ -1027,3 +1028,50 @@ mod multiline_omit {

test_emit!(rich_no_color);
}

mod highlight {
use super::*;

lazy_static::lazy_static! {
static ref TEST_DATA: TestData<'static, SimpleFile<&'static str, String>> = {
let file = SimpleFile::new(
"test.txt",
[
"This is some text.",
"\tThis is some indented text.",
"\tThis is some indented text with trailing whitespace. \t",
"This is some multiline text.",
"This is some more multiline text.",
]
.join("\n"),
);

let diagnostics = vec![
Diagnostic::note()
.with_message("not highlighted")
.with_labels(vec![
Label::primary((), 0..18),
Label::primary((), 20..47),
Label::primary((), 49..101),
]),
Diagnostic::note()
.with_message("also not highlighted")
.with_labels(vec![
Label::primary((), 19..47),
Label::primary((), 48..103),
]),
Diagnostic::note()
.with_message("highlighted")
.with_labels(vec![
Label::primary((), 0..17),
Label::primary((), 21..47),
Label::primary((), 49..53),
Label::primary((), 104..166),
]),
];
TestData{files: file, diagnostics }
};
}

test_emit!(rich_color);
}