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: Ralf Wildenhues
Subject: Re: [PATCH] check: don't use multi-line coloring for the report
Date: Mon, 20 Jun 2011 22:29:02 +0200

* Stefano Lattarini wrote on Mon, Jun 20, 2011 at 10:33:53AM CEST:
> OK, I've amended the patch on Bert's behalf, as he can't do that himself
> at the moment.  Attached is what I've pushed (to maint).

Thanks for handling this, and to Bert for the report and patch!

Minor nit:

> --- a/lib/am/check.am
> +++ b/lib/am/check.am
> @@ -75,15 +75,21 @@ am__rst_title   = sed 's/.*/   &   
> /;h;s/./=/g;p;x;p;g;p;s/.*//'
>  am__rst_section = sed 'p;s/./=/g;p;g'
>  
>  # Put stdin (possibly several lines separated by ".  ") in a box.
> -am__text_box = $(AWK) '{                             \
> -  n = split($$0, lines, "\\.  "); max = 0;           \
> -  for (i = 1; i <= n; ++i)                           \
> -    if (max < length(lines[i]))                              \
> -      max = length(lines[i]);                                \
> -  for (i = 0; i < max; ++i) line = line "=";         \
> -  print line;                                                \
> -  for (i = 1; i <= n; ++i) if (lines[i]) print lines[i];\
> -  print line;                                                \
> +# Prefix each line by 'col' and terminate each with 'std', for coloring.
> +# Multi line coloring is problematic with "less -R", so we really need
> +# to color each line individually.

Inconsistent comment level, when compared with below.  I'd use '#' here,
as this is not really interesting to anyone reading Makefile.in files.
I acknowledge that they are undercommented for many users, but then again,
getting them up to a comment level easily understandable for everyone
would just blow their size up a real lot, so a better recommendation is
to look at the .am files.  And make sure they are commented well, but not
too repetitious.

> @@ -398,14 +403,17 @@ 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"; \
> +## Multi line coloring is problematic with "less -R", so we really need
> +## to color each line individually.
> +       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

Thanks,
Ralf



reply via email to

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