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
Changes from 1 commit
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
Prev Previous commit
Next Next commit
better highlighting
  • Loading branch information
Johann150 committed Jul 12, 2020
commit 05a2559adb4e963d976241259b17768674e69f97
57 changes: 42 additions & 15 deletions codespan-reporting/src/term/renderer.rs
Original file line number Diff line number Diff line change
@@ -244,6 +244,27 @@ 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());

// count the number of whitespace characters
let text_start = source.chars().take_while(|c| c.is_whitespace()).count();

// get the byte index of the first non-whitespace character
let text_start = source
.char_indices()
.nth(text_start)
.map(|(index, _)| index)
.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
@@ -276,37 +297,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(range) => column_range.start >= range.end,
MultiLabel::TopLeft | MultiLabel::Left => true,
MultiLabel::Bottom(range, _) => column_range.end <= range.end,
}
// 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()?;
}
write!(self, "\n")?;