automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] check: don't use multi-line coloring for the report


From: Stefano Lattarini
Subject: Re: [PATCH] check: don't use multi-line coloring for the report
Date: Thu, 16 Jun 2011 09:41:30 +0200
User-agent: KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; )

Hi Bert, thanks for the patch.

On Thursday 16 June 2011, Bert Wesarg wrote:
> less can't handle coloring which spans newlines because of performance
> reasons. Thus, color each line of the check report by its own.
> 
> ---
> 
> For reference, git had a similar problem and I talked to the less
> maintainer about the problem. Here is the resulting fix in git:
> 
> http://repo.or.cz/w/git.git/commit/374664478f204ab45bbd494ab21492f331d8b1f0
> ---
>  lib/am/check.am |   13 +++++++------
>  1 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/am/check.am b/lib/am/check.am
> index 97ecb68..63451b7 100644
> --- a/lib/am/check.am
> +++ b/lib/am/check.am
> @@ -401,14 +401,15 @@ check-TESTS: $(TESTS)
>         fi; \
>         dashes=`echo "$$dashes" | sed s/./=/g`; \
>         if test "$$failed" -eq 0; then \
> -         echo "$$grn$$dashes"; \
> +         col="$$grn"; \
>         else \
> -         echo "$$red$$dashes"; \
> +         col="$$red"; \
>         fi; \
> -       echo "$$banner"; \
> -       test -z "$$skipped" || echo "$$skipped"; \
> -       test -z "$$report" || echo "$$report"; \
> -       echo "$$dashes$$std"; \
> +       echo "$${col}$$dashes$${std}"; \
> +       echo "$${col}$$banner$${std}"; \
> +       test -z "$$skipped" || echo "$${col}$$skipped$${std}"; \
> +       test -z "$$report" || echo "$${col}$$report$${std}"; \
> +       echo "$${col}$$dashes$${std}"; \
>         test "$$failed" -eq 0; \
>       else :; fi
>  
 
I verified that the problem you reported is really present, and
I agree it would be nice to fix it.

However, your patch fixes the problem only fot the old "serial"
testsuite harness, not for the new "parallel" one.  It would be
nice to fix the problem for both (especially because most
Automake-using GNU projects employs the new "parallel" harness
now).  Are you interested in improving your patch to do so?  If
yes, note that ideally you should also add yourself to the
THANKS file, and write a ChangeLog entry describing the problem
you've fixed and the changes you made to the files (if you don't,
I can write it for you before pushing the patch). 

Regards,
  Stefano



reply via email to

[Prev in Thread] Current Thread [Next in Thread]