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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 10, 2024, 08:14 UTC
Message-ID: <CAPig+cQZhrG+0BJkDbmKY11jxSspod2Xp8tSQq-DGOO9qMbR_w@mail.gmail.com>
In-Reply-To: <Zt_5zMiu4QRka5x3@pks.im>

On Tue, Sep 10, 2024 at 3:48 AM Patrick Steinhardt <ps@pks.im> wrote:

Show 17 quoted lines

> On Tue, Sep 10, 2024 at 12:10:13AM -0400, Eric Sunshine wrote:
> > [...]
> > 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.
> > Together with the preceding change which gave all problem annotations a
> > uniform "LINT:" prefix, the noisy "?!" decoration has become superfluous
> > as a search "needle" so omit it when output is colored.
> >
> > Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> > ---
> > +     my ($erropen, $errclose) = -t 1 ? ("$c->{rev}$c->{red}", $c->{reset}) : ('?!', '?!');
>
> I was first wondering why we didn't have to change our tests. But this
> seems to use either coloring or the `?!` decorations based on whether or
> not we output to a terminal. And as our tests output to a non-terminal
> they indeed see the old format, and as such they don't have to change.

Correct.

> One thing I don't like about this is that we now have different output
> depending on whether or not you happen to pipe output to e.g. less(1),
> which I do quite frequently. So I'd propose to just drop the markers
> unconditionally.

My knee-jerk reaction is that the "?!" decoration is still handy for drawing the eye when scanning non-colored output visually (not using a search feature), so I'm hesitant to drop it. However, on reflection, I'm not sure I feel very strongly about it. What do others think?