Thread (19 messages) 19 messages, 4 authors, 2024-09-10

Re: [PATCH 2/2] chainlint: reduce annotation noise-factor

From: Patrick Steinhardt <hidden>
Date: 2024-08-29 10:03:40

On Thu, Aug 29, 2024 at 05:16:25AM -0400, Eric Sunshine wrote:
From: Eric Sunshine <redacted>

When chainlint detects a problem in a test definition, it highlights the
offending code with an "?!ERR ...?!" annotation. The rather curious "?!"
delimiter was chosen to draw the reader's attention to the problem area.

Later, chainlint learned to color its output when sent to a terminal.
Problem annotations are colored with a red background which stands out
well from surrounding text, thus easily draws the reader's attention. As
such, the additional "?!" decoration became superfluous (when output is
colored), however the decoration was retained since it serves as a good
needle when using the terminal's search feature to "jump" to the next
problem.

Nevertheless, the "?!" decoration is noisy and ugly and makes it
unnecessarily difficult for the reader to pluck the problem description
from the annotation. For instance, it is easier to see at a glance what
the problem is in:

    ERR missing '&&'

than in the noisier:

    ?!ERR missing '&&'?!

Therefore drop the "!?" decoration when output is colored (but retain it
otherwise).

Note that the preceding change gave all problem annotations a uniform
"ERR" prefix which serves as a reasonably suitable replacement needle
when searching in a terminal, so loss of "?!" in the output should not
be overly problematic.
Okay, now the "ERR" prefix becomes a bit more important because we drop
the other punctuation. I'm still not much of a fan of it, though. Makes
me wonder whether we want to take a clue from how compilers nowadays
format this, e.g. by using "pointers".

So this:

    2 (
    3 	foo |
    4 	bar |
    5 	baz &&
    6 
    7 	fish |
    8 	cow ?!AMP?!
    9 
    10 	sunder
    11 )

Would become this:

    t/chainlint/pipe.actual:8: error: expected ampersands (&&)
    7 	fish |
    8 	cow 
            ^
    9

While this would be neat, I guess it would also be way more work than
the current series you have posted. And whether that work is ultimately
really worth it may be another question. Probably not.

So overall, I'm fine with the direction that your patch series takes.

Patrick
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help