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

rendering bug on small spans in large spans #316

Merged
merged 3 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
69 changes: 60 additions & 9 deletions src/handlers/graphical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ This printer can be customized by using [`new_themed()`](GraphicalReportHandler:

See [`set_hook()`](crate::set_hook) for more details on customizing your global
printer.
*/
*/
#[derive(Debug, Clone)]
pub struct GraphicalReportHandler {
pub(crate) links: LinkStyle,
Expand Down Expand Up @@ -468,7 +468,7 @@ impl GraphicalReportHandler {
for line in &lines {
let mut num_highlights = 0;
for hl in &labels {
if !line.span_line_only(hl) && line.span_applies(hl) {
if !line.span_line_only(hl) && line.span_applies_gutter(hl) {
num_highlights += 1;
}
}
Expand Down Expand Up @@ -674,7 +674,7 @@ impl GraphicalReportHandler {
}
let chars = &self.theme.characters;
let mut gutter = String::new();
let applicable = highlights.iter().filter(|hl| line.span_applies(hl));
let applicable = highlights.iter().filter(|hl| line.span_applies_gutter(hl));
let mut arrow = false;
for (i, hl) in applicable.enumerate() {
if line.span_starts(hl) {
Expand Down Expand Up @@ -735,44 +735,78 @@ impl GraphicalReportHandler {
if max_gutter == 0 {
return Ok(());
}

// keeps track of how many colums wide the gutter is
// important for ansi since simply measuring the size of the final string
// gives the wrong result when the string contains ansi codes.
let mut gutter_cols = 0;

let chars = &self.theme.characters;
let mut gutter = String::new();
let applicable = highlights.iter().filter(|hl| line.span_applies(hl));
let applicable = highlights.iter().filter(|hl| line.span_applies_gutter(hl));
for (i, hl) in applicable.enumerate() {
if !line.span_line_only(hl) && line.span_ends(hl) {
if render_mode == LabelRenderMode::MultiLineRest {
// this is to make multiline labels work. We want to make the right amount
// of horizontal space for them, but not actually draw the lines
for _ in 0..max_gutter.saturating_sub(i) + 2 {
let horizontal_space = max_gutter.saturating_sub(i) + 2;
for _ in 0..horizontal_space {
gutter.push(' ');
}
// account for one more horizontal space, since in multiline mode
// we also add in the vertical line before the label like this:
// 2 │ ╭─▶ text
// 3 │ ├─▶ here
// · ╰──┤ these two lines
// · │ are the problem
// ^this
gutter_cols += horizontal_space + 1;
} else {
let num_repeat = max_gutter.saturating_sub(i) + 2;

gutter.push_str(&chars.lbot.style(hl.style).to_string());

gutter.push_str(
&chars
.hbar
.to_string()
.repeat(
max_gutter.saturating_sub(i)
num_repeat
// if we are rendering a multiline label, then leave a bit of space for the
// rcross character
+ if render_mode == LabelRenderMode::MultiLineFirst {
- if render_mode == LabelRenderMode::MultiLineFirst {
1
} else {
2
0
},
)
.style(hl.style)
.to_string(),
);

// we count 1 for the lbot char, and then a few more, the same number
// as we just repeated for. For each repeat we only add 1, even though
// due to ansi escape codes the number of bytes in the string could grow
// a lot each time.
gutter_cols += num_repeat + 1;
}
break;
} else {
gutter.push_str(&chars.vbar.style(hl.style).to_string());

// we may push many bytes for the ansi escape codes style adds,
// but we still only add a single character-width to the string in a terminal
gutter_cols += 1;
}
}
write!(f, "{:width$}", gutter, width = max_gutter + 1)?;

// now calculate how many spaces to add based on how many columns we just created.
// it's the max width of the gutter, minus how many character-widths we just generated
// capped at 0 (though this should never go below in reality), and then we add 3 to
// account for arrowheads when a gutter line ends
let num_spaces = (max_gutter + 3).saturating_sub(gutter_cols);
// we then write the gutter and as many spaces as we need
write!(f, "{}{:width$}", gutter, "", width = num_spaces)?;
Ok(())
}

Expand Down Expand Up @@ -1133,16 +1167,33 @@ impl Line {
span.offset() >= self.offset && span.offset() + span.len() <= self.offset + self.length
}

/// Returns whether `span` should be visible on this line, either in the gutter or under the
/// text on this line
fn span_applies(&self, span: &FancySpan) -> bool {
let spanlen = if span.len() == 0 { 1 } else { span.len() };
// Span starts in this line

(span.offset() >= self.offset && span.offset() < self.offset + self.length)
// Span passes through this line
|| (span.offset() < self.offset && span.offset() + spanlen > self.offset + self.length) //todo
// Span ends on this line
|| (span.offset() + spanlen > self.offset && span.offset() + spanlen <= self.offset + self.length)
}

/// Returns whether `span` should be visible on this line in the gutter (so this excludes spans
/// that are only visible on this line and do not span multiple lines)
fn span_applies_gutter(&self, span: &FancySpan) -> bool {
let spanlen = if span.len() == 0 { 1 } else { span.len() };
// Span starts in this line
self.span_applies(span)
&& !(
// as long as it doesn't start *and* end on this line
(span.offset() >= self.offset && span.offset() < self.offset + self.length)
&& (span.offset() + spanlen > self.offset
&& span.offset() + spanlen <= self.offset + self.length)
)
}

// A 'flyby' is a multi-line span that technically covers this line, but
// does not begin or end within the line itself. This method is used to
// calculate gutters.
Expand Down
6 changes: 3 additions & 3 deletions tests/graphical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,9 @@ if true {
let expected = r#" × oops!
╭─[issue:1:1]
1 │ ╭─▶ if true {
2 │ │╭▶ a
· │
· │ ╰── small
2 │ │ a
· │
· │ ╰── small
3 │ │ } else {
4 │ │ b
5 │ ├─▶ }
Expand Down