Re: [PATCH 1/2] chainlint: make error messages self-explanatory

From: Jeff King <peff@peff.net>
Date: Aug 29, 2024, 17:07 UTC
Message-ID: <20240829170712.GA405209@coredump.intra.peff.net>
In-Reply-To: <ZtBHbftK7vdTEz93@tanuki>

On Thu, Aug 29, 2024 at 12:03:33PM +0200, Patrick Steinhardt wrote:

Show 17 quoted lines

> > diff --git a/t/chainlint/arithmetic-expansion.expect b/t/chainlint/arithmetic-expansion.expect
> > index 338ecd5861..2efd65dcbd 100644
> > --- a/t/chainlint/arithmetic-expansion.expect
> > +++ b/t/chainlint/arithmetic-expansion.expect
> > @@ -4,6 +4,6 @@
> >  5 	baz
> >  6 ) &&
> >  7 (
> > -8 	bar=$((42 + 1)) ?!AMP?!
> > +8 	bar=$((42 + 1)) ?!ERR missing '&&'?!
> >  9 	baz
> >  10 )
> 
> I find the resulting error messages a bit confusing: to me it reads as
> if "ERR" is missing the ampersands. Is it actually useful to have the
> ERR prefix in the first place? We do not output anything but errors, so
> it feels somewhat redundant.

I wonder if coloring "ERR" differently, or perhaps even adding a colon, like "ERR: ", would make it stand out more.

FWIW, I find the existing error messages pretty readable, but that is probably a sign that my mind has been poisoned by using chainlint too much already. ;)

-Peff