Re: [PATCH v2 3/3] chainlint: reduce annotation noise-factor
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Sep 10, 2024, 07:48 UTC
- Message-ID
- <Zt_5zMiu4QRka5x3@pks.im>
- In-Reply-To
- <20240910041013.68948-4-ericsunshine@charter.net>
On Tue, Sep 10, 2024 at 12:10:13AM -0400, Eric Sunshine wrote:
Show 33 quoted lines
> From: Eric Sunshine <sunshine@sunshineco.com>
>
> When chainlint detects a problem in a test definition, it highlights the
> offending code with a "?!...?!" annotation. The rather curious "?!"
> decoration was chosen to draw the reader's attention to the problem area
> and to act as a good "needle" when using the terminal's search feature
> to "jump" to the next problem.
>
> 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>
> ---
> t/chainlint.pl | 3 ++-
> t/test-lib.sh | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/t/chainlint.pl b/t/chainlint.pl
> index ad26499478..f0598e3934 100755
> --- a/t/chainlint.pl
> +++ b/t/chainlint.pl
> @@ -651,6 +651,7 @@ sub check_test {
> $self->{nerrs} += @$problems;
> return unless $emit_all || @$problems;
> my $c = main::fd_colors(1);
> + my ($erropen, $errclose) = -t 1 ? ("$c->{rev}$c->{red}", $c->{reset}) : ('?!', '?!');
> my $start = 0;
> my $checked = '';
> for (sort {$a->[1]->[2] <=> $b->[1]->[2]} @$problems) {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.
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.
Patrick