GNU bug report logs -
#20062
[PATCH] diff: add support for --color
Previous Next
Full log
View this message in rfc822 format
[Message part 1 (text/plain, inline)]
On 11/03/2015 10:05 AM, Giuseppe Scrivano wrote:
> I have attached the patches that implement --palette, the missing tests
> and update the NEWS file.
>
> +++ b/doc/diffutils.texi
> @@ -3763,6 +3763,7 @@ Always use color.
> Specifying @option{--color} and no @var{when} is equivalent to
> @option{--color=auto}.
>
> +
> @item -C @var{lines}
Spurious change?
> @itemx --context <at> r{[}=@var{lines}@r{]}
> Use the context output format, showing @var{lines} (an integer) lines of
> @@ -3890,6 +3891,11 @@ if-then-else format. @xref{Line Formats}.
> @itemx --show-c-function
> Show which C function each change is in. @xref{C Function Headings}.
>
> +@item --palette=@var{scheme}
> +It allows to specify what colors are used to colorize the output. It
Passive voice. Would sound better as:
Specify what color palette to use when colored output to use.
> +defaults to @samp{rs=0:hd=1:ad=32:de=31:ln=36} for red added lines,
> +green deleted lines, cyan line numbers, bold header.
That says the default, but doesn't say what conventions to apply to get
something other than the default. What are 'rs', 'hd', 'ad', 'de', and
'ln'? Can we link to the fact that the values of these variables are
generally applied inside a terminal '\e[Xm' sequence, for a given X?
Can we link to dircolors output for comparison? Do we need parameters
for the '\e[' and 'm' prefix/suffix, in case the terminal understands
something different than ANSI escapes for colors?
> @@ -950,6 +956,7 @@ static char const * const option_help_msgid[] = {
> N_(" --speed-large-files assume large files and many scattered small changes"),
> N_(" --color[=WHEN] colorize the output; WHEN can be 'never', 'always',"),
> N_(" or 'auto' (the default)"),
> + N_(" --palette=SCHEME specify the colors to use when --color is active"),
> "",
> N_(" --help display this help and exit"),
> N_("-v, --version output version information and exit"),
Where is SCHEME documented in --help? Should it be?
> +
> +/* Parse a string as part of the DIFF_COLORS variable; this may involve
Stale reference to envvar instead of --palette.
> +
> +static struct bin_str color_indicator[] =
> + {
> + { LEN_STR_PAIR ("\033[") }, /* lc: Left of color sequence */
> + { LEN_STR_PAIR ("m") }, /* rc: Right of color sequence */
> + { 0, NULL }, /* ec: End color (replaces lc+rs+rc) */
> + { LEN_STR_PAIR ("0") }, /* rs: Reset to ordinary colors */
> + { LEN_STR_PAIR ("1") }, /* hd: Header */
> + { LEN_STR_PAIR ("32") }, /* ad: Add line */
> + { LEN_STR_PAIR ("31") }, /* de: Delete line */
> + { LEN_STR_PAIR ("36") }, /* ln: Line number */
> + };
This needs to be documented in more than just code comments.
> + ext = NULL;
> + strcpy (label, "??");
> +
> + /* This is an overly conservative estimate, but any possible
> + DIFF_COLORS string will *not* generate a color_buf longer than
> + itself, so it is a safe way of allocating a buffer in
> + advance. */
> + buf = color_buf = xstrdup (p);
Another stale ref to DIFF_COLORS.
> 0002-doc-mention-color-and-palette-in-NEWS.patch
>
Looked okay to me.
> 0003-tests-Add-tests-for-color-and-palette.patch
>
> +
> +# Compare with some known outputs
> +
> +diff -c --color=always a b | sha1sum \
> + | grep 904a91f82474e3532459b759fdacbdb339070e14 || fail=1
> +
> +diff -N --color=always --palette="rs=0:hd=33:ad=34:de=35:ln=36" a b \
> + | sha1sum | grep 7796a82c2e7bd1f4ee04cb44352d83e1db87c092 || fail=1
Nice; a test of a non-default palette.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[signature.asc (application/pgp-signature, attachment)]
This bug report was last modified 8 years and 78 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.