Skip to content

Commit

Permalink
fix(graphical): prevent leading newline when no link/code (#418)
Browse files Browse the repository at this point in the history
  • Loading branch information
@unbyte
unbyte authored Jan 14, 2025
1 parent 771a075 commit 1e1938a
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 31 deletions.
18 changes: 14 additions & 4 deletions src/handlers/graphical.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ impl GraphicalReportHandler {
parent_src: Option<&dyn SourceCode>,
) -> fmt::Result {
let src = diagnostic.source_code().or(parent_src);
self.render_header(f, diagnostic)?;
self.render_header(f, diagnostic, false)?;
self.render_causes(f, diagnostic, src)?;
self.render_snippets(f, diagnostic, src)?;
self.render_footer(f, diagnostic)?;
Expand All @@ -261,13 +261,19 @@ impl GraphicalReportHandler {
Ok(())
}

fn render_header(&self, f: &mut impl fmt::Write, diagnostic: &(dyn Diagnostic)) -> fmt::Result {
fn render_header(
&self,
f: &mut impl fmt::Write,
diagnostic: &(dyn Diagnostic),
is_nested: bool,
) -> fmt::Result {
let severity_style = match diagnostic.severity() {
Some(Severity::Error) | None => self.theme.styles.error,
Some(Severity::Warning) => self.theme.styles.warning,
Some(Severity::Advice) => self.theme.styles.advice,
};
let mut header = String::new();
let mut need_newline = is_nested;
if self.links == LinkStyle::Link && diagnostic.url().is_some() {
let url = diagnostic.url().unwrap(); // safe
let code = if let Some(code) = diagnostic.code() {
Expand All @@ -284,15 +290,19 @@ impl GraphicalReportHandler {
);
write!(header, "{}", link)?;
writeln!(f, "{}", header)?;
need_newline = true;
} else if let Some(code) = diagnostic.code() {
write!(header, "{}", code.style(severity_style),)?;
if self.links == LinkStyle::Text && diagnostic.url().is_some() {
let url = diagnostic.url().unwrap(); // safe
write!(header, " ({})", url.style(self.theme.styles.link))?;
}
writeln!(f, "{}", header)?;
need_newline = true;
}
if need_newline {
writeln!(f)?;
}
writeln!(f)?;
Ok(())
}

Expand Down Expand Up @@ -493,7 +503,7 @@ impl GraphicalReportHandler {
Some(Severity::Warning) => write!(f, "Warning: ")?,
Some(Severity::Advice) => write!(f, "Advice: ")?,
};
inner_renderer.render_header(f, rel)?;
inner_renderer.render_header(f, rel, true)?;
let src = rel.source_code().or(parent_src);
inner_renderer.render_causes(f, rel, src)?;
inner_renderer.render_snippets(f, rel, src)?;
Expand Down
40 changes: 19 additions & 21 deletions tests/graphical.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ fn word_wrap_options() -> Result<(), MietteError> {
let out =
fmt_report_with_settings(Report::msg("abcdefghijklmnopqrstuvwxyz"), |handler| handler);

let expected = "\n × abcdefghijklmnopqrstuvwxyz\n".to_string();
let expected = " × abcdefghijklmnopqrstuvwxyz\n".to_string();
assert_eq!(expected, out);

// A long word can break with a smaller width
Expand All @@ -75,14 +75,14 @@ fn word_wrap_options() -> Result<(), MietteError> {
│ uvwx
│ yz
"#
.to_string();
.trim_start_matches('\n');
assert_eq!(expected, out);

// Unless, word breaking is disabled
let out = fmt_report_with_settings(Report::msg("abcdefghijklmnopqrstuvwxyz"), |handler| {
handler.with_width(10).with_break_words(false)
});
let expected = "\n × abcdefghijklmnopqrstuvwxyz\n".to_string();
let expected = " × abcdefghijklmnopqrstuvwxyz\n".to_string();
assert_eq!(expected, out);

// Breaks should start at the boundary of each word if possible
Expand All @@ -104,7 +104,7 @@ fn word_wrap_options() -> Result<(), MietteError> {
│ 5678
│ 90
"#
.to_string();
.trim_start_matches('\n');
assert_eq!(expected, out);

// But long words should not break if word breaking is disabled
Expand All @@ -121,7 +121,7 @@ fn word_wrap_options() -> Result<(), MietteError> {
│ 1234567
│ 1234567890
"#
.to_string();
.trim_start_matches('\n');
assert_eq!(expected, out);

// Unless, of course, there are hyphens
Expand Down Expand Up @@ -149,7 +149,7 @@ fn word_wrap_options() -> Result<(), MietteError> {
│ f-g-
│ h
"#
.to_string();
.trim_start_matches('\n');
assert_eq!(expected, out);

// Which requires an additional opt-out
Expand All @@ -171,7 +171,7 @@ fn word_wrap_options() -> Result<(), MietteError> {
│ a-b-c-d-e-f-g
│ a-b-c-d-e-f-g-h
"#
.to_string();
.trim_start_matches('\n');
assert_eq!(expected, out);

// Or if there are _other_ unicode word boundaries
Expand Down Expand Up @@ -199,7 +199,7 @@ fn word_wrap_options() -> Result<(), MietteError> {
│ f/g/
│ h
"#
.to_string();
.trim_start_matches('\n');
assert_eq!(expected, out);

// Such things require you to opt-in to only breaking on ASCII whitespace
Expand All @@ -221,7 +221,7 @@ fn word_wrap_options() -> Result<(), MietteError> {
│ a/b/c/d/e/f/g
│ a/b/c/d/e/f/g/h
"#
.to_string();
.trim_start_matches('\n');
assert_eq!(expected, out);

Ok(())
Expand All @@ -245,7 +245,7 @@ fn wrap_option() -> Result<(), MietteError> {
│ pqr stu
│ vwx yz
"#
.to_string();
.trim_start_matches('\n');
assert_eq!(expected, out);

// Unless, wrapping is disabled
Expand All @@ -254,7 +254,7 @@ fn wrap_option() -> Result<(), MietteError> {
|handler| handler.with_width(15).with_wrap_lines(false),
);
let expected =
"\n × abc def ghi jkl mno pqr stu vwx yz abc def ghi jkl mno pqr stu vwx yz\n".to_string();
" × abc def ghi jkl mno pqr stu vwx yz abc def ghi jkl mno pqr stu vwx yz\n".to_string();
assert_eq!(expected, out);

// Then, user-defined new lines should be preserved wrapping is disabled
Expand All @@ -267,7 +267,7 @@ fn wrap_option() -> Result<(), MietteError> {
│ abc def ghi jkl mno pqr stu vwx yz
│ abc def ghi jkl mno pqr stu vwx yz
"#
.to_string();
.trim_start_matches('\n');
assert_eq!(expected, out);

Ok(())
Expand Down Expand Up @@ -485,8 +485,7 @@ if true {
· ╰──── big
╰────
"#
.to_string();

.trim_start_matches('\n');
assert_eq!(expected, out);
}

Expand Down Expand Up @@ -517,8 +516,7 @@ fn single_line_highlight_span_full_line() {
· ╰── This bit here
╰────
"#
.to_string();

.trim_start_matches('\n');
assert_eq!(expected, out);
}

Expand Down Expand Up @@ -1781,8 +1779,7 @@ fn zero_length_eol_span() {
· ╰── This bit here
╰────
"#
.to_string();

.trim_start_matches('\n');
assert_eq!(expected, out);
}

Expand Down Expand Up @@ -1817,8 +1814,7 @@ fn primary_label() {
· ╰── nope
╰────
"#
.to_string();

.trim_start_matches('\n');
assert_eq!(expected, out);
}

Expand Down Expand Up @@ -1968,7 +1964,8 @@ fn syntax_highlighter() {
· ╰── this is a label
3 │ }
╰────
"#;
"#
.trim_start_matches('\n');
assert!(out.contains("\u{1b}[38;2;180;142;173m"));
assert_eq!(expected, strip_ansi_escapes::strip_str(out))
}
Expand Down Expand Up @@ -2028,6 +2025,7 @@ fn syntax_highlighter_on_real_file() {
l2 = line,
l3 = line + 1
);
let expected = expected.trim_start_matches('\n');
assert!(out.contains("\u{1b}[38;2;180;142;173m"));
assert_eq!(expected, strip_ansi_escapes::strip_str(out));
}
Expand Down
19 changes: 13 additions & 6 deletions tests/test_diagnostic_source_macro.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ fn test_diagnostic_source_pass_extra_info() {
this is a footer
"#
.to_string();
.trim_start_matches('\n');

assert_eq!(expected, out);
}

Expand Down Expand Up @@ -149,7 +150,8 @@ fn test_diagnostic_source_is_output() {
╰────
help: That's where the error is!
"#;
"#
.trim_start_matches('\n');

assert_eq!(expected, out);
}
Expand Down Expand Up @@ -208,7 +210,8 @@ fn test_nested_diagnostic_source_is_output() {
╰────
Yooo, a footer
"#;
"#
.trim_start_matches('\n');

assert_eq!(expected, out);
}
Expand Down Expand Up @@ -295,7 +298,8 @@ fn test_nested_cause_chains_for_related_errors_are_output() {
╰────
Yooo, a footer
"#;
"#
.trim_start_matches('\n');

assert_eq!(expected, out);
}
Expand Down Expand Up @@ -354,7 +358,9 @@ fn test_display_related_errors_as_nested() {
· ╰── here
╰────
help: Get a grip...
"#;
"#
.trim_start_matches('\n');

assert_eq!(expected, out);
}

Expand Down Expand Up @@ -403,7 +409,8 @@ fn source_is_inherited_to_causes() {
Yooo, a footer
"#;
"#
.trim_start_matches('\n');

assert_eq!(expected, out);
}

0 comments on commit 1e1938a

Please sign in to comment.