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

Trailing whitespace characters #52

Open
max-sixty opened this issue Nov 29, 2022 · 2 comments
Open

Trailing whitespace characters #52

max-sixty opened this issue Nov 29, 2022 · 2 comments

Comments

@max-sixty
Copy link
Sponsor

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: PRQL/prql@80ae431, from PRQL/prql#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
Copy link
Owner

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
Copy link
Sponsor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants