zesterer / ariadne

A fancy diagnostics & error reporting crate
https://crates.io/crates/ariadne
MIT License
1.61k stars 72 forks source link

Trailing whitespace characters #52

Open max-sixty opened 1 year ago

max-sixty commented 1 year ago

Firstly thank you for the excellent library! We're heavily using it in PRQL.

In PRQL we have a pre-commit action which removes trailing whitespace. It seems that ariadne's outputs have trailing whitespace, so inline snapshot tests can't pass. Here's an example: https://github.com/prql/prql/commit/80ae4319e1558e81025f6f72fe5f87f851e6fcda, from https://github.com/prql/prql/pull/1181.

Would it be reasonable to strip trailing whitespace? Or is that too narrow a goal? It's not that critical for us; we can move to using snapshot tests in .snap files if needed.

Thanks again

zesterer commented 1 year ago

I'm happy to accept the PR that implements this, although unfortunately I'm probably not going to have the time to commit to implementing it myself for at least a few weeks.

max-sixty commented 1 year ago

I had a go for an hour but didn't quite get there; let me post what I did — if I'm close then possibly I can have another go at the weekend.

This is the standard "cargo run --example multifile output, but with the spaces replaced with x to make them more obvious.

We want to remove the xs from the ending of the lines.

   ╭─[b.tao:1:11]
   │
 1 │ def six = five + "1"
   · xxxxxxxxxx──┬─x┬x─┬─xx
   ·             ╰───────── This is of type Nat
   · xxxxxxxxxxxxxxx│xx│xxx
   ·                │  ╰─── This is of type Str
   · xxxxxxxxxxxxxxx│xxxxxx
   ·                ╰──────  Nat and Str undergo addition here
   │
   ├─[a.tao:1:5]
   │
 1 │ def five = 5
   · xxxx──┬─xx
   ·       ╰─── Original definition of five is here
   ·
   · Note: Nat is a number and can only be added to other numbers
───╯

(here's the code I used to get the xs)

diff --git a/src/write.rs b/src/write.rs
index 9326f84..b11d245 100644
--- a/src/write.rs
+++ b/src/write.rs
@@ -506,7 +506,7 @@ struct LineLabel<'a, S> {
                             } else if let Some(underline_ll) = underline {
                                 [draw.underline.fg(underline_ll.label.color); 2]
                             } else {
-                                [' '.fg(None); 2]
+                                ['x'.fg(None); 2]
                             };

                             for i in 0..width {

By adjusting the arrow line length, we can remove some of the endings, but a) that's doing too much and b) it doesn't remove the others.

diff --git a/src/write.rs b/src/write.rs
index 9326f84..a0785ee 100644
--- a/src/write.rs
+++ b/src/write.rs
@@ -411,7 +411,8 @@ struct LineLabel<'a, S> {
                 line_labels.sort_by_key(|ll| (ll.label.order, ll.col, !ll.label.span.start()));

                 // Determine label bounds so we know where to put error messages
-                let arrow_end_space = if self.config.compact { 1 } else { 2 };
+                let arrow_end_space = 0;
                 let arrow_len = line_labels
                     .iter()
                     .fold(0, |l, ll| if ll.multi {
   ╭─[b.tao:1:11]
   │
 1 │ def six = five + "1"
   · xxxxxxxxxx──┬─x┬x─┬─
   ·             ╰─────── This is of type Nat
   · xxxxxxxxxxxxxxx│xx│x
   ·                │  ╰─ This is of type Str
   · xxxxxxxxxxxxxxx│xxxx
   ·                ╰────  Nat and Str undergo addition here
   │
   ├─[a.tao:1:5]
   │
 1 │ def five = 5
   · xxxx──┬─
   ·       ╰─ Original definition of five is here
   ·
   · Note: Nat is a number and can only be added to other numbers
───╯

The main issue seems to be that we need to know in the loop whether we've finished writing the significant characters, and I wasn't sure of a reasonable way of doing that.

Ofc the alternative is to do another pass at the end, but that seems like a very clunky solution for a relatively small problem.