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

[Eric Sunshine <ericsunshine@xxxxxxxxxxx>]

From: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>

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.

Signed-off-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
---
 t/chainlint.pl | 7 +++++--
 t/test-lib.sh  | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/chainlint.pl b/t/chainlint.pl
index d79f183dfd..971ab9212a 100755
--- a/t/chainlint.pl
+++ b/t/chainlint.pl
@@ -591,6 +591,7 @@ sub new {
 	my $class = shift @_;
 	my $self = $class->SUPER::new(@_);
 	$self->{ntests} = 0;
+	$self->{nerrs} = 0;
 	return $self;
 }
 
@@ -647,8 +648,10 @@ sub check_test {
 	my $parser = TestParser->new(\$body);
 	my @tokens = $parser->parse();
 	my $problems = $parser->{problems};
+	$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) {
@@ -663,7 +666,7 @@ sub check_test {
 	$checked =~ s/^\d+ \n//;
 	$checked =~ s/(\s) \?!/$1?!/mg;
 	$checked =~ s/\?! (\s)/?!$1/mg;
-	$checked =~ s/(\?![^?]+\?!)/$c->{rev}$c->{red}$1$c->{reset}/mg;
+	$checked =~ s/\?!([^?]+)\?!/$erropen$1$errclose/mg;
 	$checked =~ s/^\d+/$c->{dim}$&$c->{reset}/mg;
 	$checked .= "\n" unless $checked =~ /\n$/;
 	push(@{$self->{output}}, "$c->{blue}# chainlint: $title$c->{reset}\n$checked");
@@ -805,9 +808,9 @@ sub check_script {
 			my $c = fd_colors(1);
 			my $s = join('', @{$parser->{output}});
 			$emit->("$c->{bold}$c->{blue}# chainlint: $path$c->{reset}\n" . $s);
-			$nerrs += () = $s =~ /\?![^?]+\?!/g;
 		}
 		$ntests += $parser->{ntests};
+		$nerrs += $parser->{nerrs};
 	}
 	return [$id, $nscripts, $ntests, $nerrs];
 }
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 54247604cb..b652cb98cd 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1606,7 +1606,7 @@ if test "${GIT_TEST_CHAIN_LINT:-1}" != 0 &&
    test "${GIT_TEST_EXT_CHAIN_LINT:-1}" != 0
 then
 	"$PERL_PATH" "$TEST_DIRECTORY/chainlint.pl" "$0" ||
-		BUG "lint error (see '?!...!? annotations above)"
+		BUG "lint error (see 'ERR' annotations above)"
 fi
 
 # Last-minute variable setup
-- 
2.46.0