Jim Meyering <j...@meyering.net> writes:
> On Mon, Nov 2, 2015 at 11:34 AM, Eric Blake <ebl...@redhat.com> wrote:
>> On 11/02/2015 12:17 PM, Giuseppe Scrivano wrote:
>>
>>> Anyway, I would like an opinion about the new name before I send te full
>>> series again. I avoided --colors as it is very similar to --color, but
>>> I am not sure how --color-scheme sounds for a native speaker.
>>
>> --color-scheme sounds okay to a native, but has the issue that it cannot
>> be abbreviated (--col would be short for --color, not --color-scheme).
>> Can you come up with a name that does not share a common prefix? Maybe
>> --palette?
>
> Good point.
> I too prefer --palette='...'
> That does have the tiny downside that diff's rarely used
> --paginate option will no longer be able to be abbreviated
> as "--pa", but that is barely worth mentioning.
I have attached the patches that implement --palette, the missing tests
and update the NEWS file.
Regards,
Giuseppe
>From f7bf589e9a0c4b893837cfbbdaae5543d72c2b85 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <gscriv...@gnu.org>
Date: Mon, 19 Oct 2015 10:29:41 +0200
Subject: [PATCH 1/3] diff: add --palette
* doc/diffutils.texi: Add documentation for --palette
* src/diff.h (set_color_palette): New prototype.
* src/diff.c (set_color_palette): New function.
(color_palette): New variable.
* src/utils.c (struct bin_str): New struct.
(struct color_ext_type): New struct.
(color_indicator): New array.
(indicator_name): New array.
(indicator_no): New enum.
(parse_state): New enum.
(put_indicator): New function.
(get_funky_string): New function. Copied from coreutils ls.
(parse_diff_color): New function. Copied from coreutils ls
"parse_ls_color" function.
(set_header_color_context): Use put_indicator instead of directly
outputting the sequence.
(set_line_numbers_colors_context): Likewise.
(set_add_color_context): Likewise.
(set_delete_color_context): Likewise.
(reset_color_context): Likewise.
---
doc/diffutils.texi | 6 +
src/diff.c | 7 +
src/diff.h | 1 +
src/util.c | 439 +++++++++++++++++++++++++++++++++++++++++++++++++++--
4 files changed, 444 insertions(+), 9 deletions(-)
diff --git a/doc/diffutils.texi b/doc/diffutils.texi
index 0944b44..b689673 100644
--- a/doc/diffutils.texi
+++ 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}
@itemx --context@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
+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.
+
@item -q
@itemx --brief
Report only whether the files differ, not the details of the
diff --git a/src/diff.c b/src/diff.c
index 4e0f602..f5298e1 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -140,6 +140,7 @@ enum
CHANGED_GROUP_FORMAT_OPTION,
COLOR_OPTION,
+ COLOR_PALETTE_OPTION,
};
static char const group_format_option[][sizeof "--unchanged-group-format"] =
@@ -196,6 +197,7 @@ static struct option const longopts[] =
{"old-group-format", 1, 0, OLD_GROUP_FORMAT_OPTION},
{"old-line-format", 1, 0, OLD_LINE_FORMAT_OPTION},
{"paginate", 0, 0, 'l'},
+ {"palette", 1, 0, COLOR_PALETTE_OPTION},
{"rcs", 0, 0, 'n'},
{"recursive", 0, 0, 'r'},
{"report-identical-files", 0, 0, 's'},
@@ -635,6 +637,10 @@ main (int argc, char **argv)
specify_colors_style (optarg);
break;
+ case COLOR_PALETTE_OPTION:
+ set_color_palette (optarg);
+ break;
+
default:
try_help (NULL, NULL);
}
@@ -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"),
diff --git a/src/diff.h b/src/diff.h
index 472fa93..5930cd1 100644
--- a/src/diff.h
+++ b/src/diff.h
@@ -411,3 +411,4 @@ extern void set_add_color_context (void);
extern void set_delete_color_context (void);
extern void reset_color_context (void);
extern void set_line_numbers_color_context (void);
+extern void set_color_palette (const char *palette);
diff --git a/src/util.c b/src/util.c
index 6cc1411..50dce82 100644
--- a/src/util.c
+++ b/src/util.c
@@ -310,6 +310,396 @@ static char const *current_name1;
static bool currently_recursive;
static bool colors_enabled;
+static struct color_ext_type *color_ext_list = NULL;
+
+struct bin_str
+ {
+ size_t len; /* Number of bytes */
+ const char *string; /* Pointer to the same */
+ };
+
+struct color_ext_type
+ {
+ struct bin_str ext; /* The extension we're looking for */
+ struct bin_str seq; /* The sequence to output when we do */
+ struct color_ext_type *next; /* Next in list */
+ };
+
+/* Parse a string as part of the DIFF_COLORS variable; this may involve
+ decoding all kinds of escape characters. If equals_end is set an
+ unescaped equal sign ends the string, otherwise only a : or \0
+ does. Set *OUTPUT_COUNT to the number of bytes output. Return
+ true if successful.
+
+ The resulting string is *not* null-terminated, but may contain
+ embedded nulls.
+
+ Note that both dest and src are char **; on return they point to
+ the first free byte after the array and the character that ended
+ the input string, respectively. */
+
+static bool
+get_funky_string (char **dest, const char **src, bool equals_end,
+ size_t *output_count)
+{
+ char num; /* For numerical codes */
+ size_t count; /* Something to count with */
+ enum {
+ ST_GND, ST_BACKSLASH, ST_OCTAL, ST_HEX, ST_CARET, ST_END, ST_ERROR
+ } state;
+ const char *p;
+ char *q;
+
+ p = *src; /* We don't want to double-indirect */
+ q = *dest; /* the whole darn time. */
+
+ count = 0; /* No characters counted in yet. */
+ num = 0;
+
+ state = ST_GND; /* Start in ground state. */
+ while (state < ST_END)
+ {
+ switch (state)
+ {
+ case ST_GND: /* Ground state (no escapes) */
+ switch (*p)
+ {
+ case ':':
+ case '\0':
+ state = ST_END; /* End of string */
+ break;
+ case '\\':
+ state = ST_BACKSLASH; /* Backslash scape sequence */
+ ++p;
+ break;
+ case '^':
+ state = ST_CARET; /* Caret escape */
+ ++p;
+ break;
+ case '=':
+ if (equals_end)
+ {
+ state = ST_END; /* End */
+ break;
+ }
+ /* else fall through */
+ default:
+ *(q++) = *(p++);
+ ++count;
+ break;
+ }
+ break;
+
+ case ST_BACKSLASH: /* Backslash escaped character */
+ switch (*p)
+ {
+ case '0':
+ case '1':
+ case '2':
+ case '3':
+ case '4':
+ case '5':
+ case '6':
+ case '7':
+ state = ST_OCTAL; /* Octal sequence */
+ num = *p - '0';
+ break;
+ case 'x':
+ case 'X':
+ state = ST_HEX; /* Hex sequence */
+ num = 0;
+ break;
+ case 'a': /* Bell */
+ num = '\a';
+ break;
+ case 'b': /* Backspace */
+ num = '\b';
+ break;
+ case 'e': /* Escape */
+ num = 27;
+ break;
+ case 'f': /* Form feed */
+ num = '\f';
+ break;
+ case 'n': /* Newline */
+ num = '\n';
+ break;
+ case 'r': /* Carriage return */
+ num = '\r';
+ break;
+ case 't': /* Tab */
+ num = '\t';
+ break;
+ case 'v': /* Vtab */
+ num = '\v';
+ break;
+ case '?': /* Delete */
+ num = 127;
+ break;
+ case '_': /* Space */
+ num = ' ';
+ break;
+ case '\0': /* End of string */
+ state = ST_ERROR; /* Error! */
+ break;
+ default: /* Escaped character like \ ^ : = */
+ num = *p;
+ break;
+ }
+ if (state == ST_BACKSLASH)
+ {
+ *(q++) = num;
+ ++count;
+ state = ST_GND;
+ }
+ ++p;
+ break;
+
+ case ST_OCTAL: /* Octal sequence */
+ if (*p < '0' || *p > '7')
+ {
+ *(q++) = num;
+ ++count;
+ state = ST_GND;
+ }
+ else
+ num = (num << 3) + (*(p++) - '0');
+ break;
+
+ case ST_HEX: /* Hex sequence */
+ switch (*p)
+ {
+ case '0':
+ case '1':
+ case '2':
+ case '3':
+ case '4':
+ case '5':
+ case '6':
+ case '7':
+ case '8':
+ case '9':
+ num = (num << 4) + (*(p++) - '0');
+ break;
+ case 'a':
+ case 'b':
+ case 'c':
+ case 'd':
+ case 'e':
+ case 'f':
+ num = (num << 4) + (*(p++) - 'a') + 10;
+ break;
+ case 'A':
+ case 'B':
+ case 'C':
+ case 'D':
+ case 'E':
+ case 'F':
+ num = (num << 4) + (*(p++) - 'A') + 10;
+ break;
+ default:
+ *(q++) = num;
+ ++count;
+ state = ST_GND;
+ break;
+ }
+ break;
+
+ case ST_CARET: /* Caret escape */
+ state = ST_GND; /* Should be the next state... */
+ if (*p >= '@' && *p <= '~')
+ {
+ *(q++) = *(p++) & 037;
+ ++count;
+ }
+ else if (*p == '?')
+ {
+ *(q++) = 127;
+ ++count;
+ }
+ else
+ state = ST_ERROR;
+ break;
+
+ default:
+ abort ();
+ }
+ }
+
+ *dest = q;
+ *src = p;
+ *output_count = count;
+
+ return state != ST_ERROR;
+}
+
+enum parse_state
+ {
+ PS_START = 1,
+ PS_2,
+ PS_3,
+ PS_4,
+ PS_DONE,
+ PS_FAIL
+ };
+
+#define LEN_STR_PAIR(s) sizeof (s) - 1, s
+
+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 */
+ };
+
+static const char *const indicator_name[]=
+ {
+ "lc", "rc", "ec", "rs", "hd", "ad", "de", "ln", NULL
+ };
+
+static const char *color_palette;
+
+void
+set_color_palette (const char *palette)
+{
+ color_palette = palette;
+}
+
+static void
+parse_diff_color (void)
+{
+ char *color_buf;
+ const char *p; /* Pointer to character being parsed */
+ char *buf; /* color_buf buffer pointer */
+ int ind_no; /* Indicator number */
+ char label[3]; /* Indicator label */
+ struct color_ext_type *ext; /* Extension we are working on */
+
+ if ((p = color_palette) == NULL || *p == '\0')
+ return;
+
+ 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);
+
+ enum parse_state state = PS_START;
+ while (true)
+ {
+ switch (state)
+ {
+ case PS_START: /* First label character */
+ switch (*p)
+ {
+ case ':':
+ ++p;
+ break;
+
+ case '*':
+ /* Allocate new extension block and add to head of
+ linked list (this way a later definition will
+ override an earlier one, which can be useful for
+ having terminal-specific defs override global). */
+
+ ext = xmalloc (sizeof *ext);
+ ext->next = color_ext_list;
+ color_ext_list = ext;
+
+ ++p;
+ ext->ext.string = buf;
+
+ state = (get_funky_string (&buf, &p, true, &ext->ext.len)
+ ? PS_4 : PS_FAIL);
+ break;
+
+ case '\0':
+ state = PS_DONE; /* Done! */
+ goto done;
+
+ default: /* Assume it is file type label */
+ label[0] = *(p++);
+ state = PS_2;
+ break;
+ }
+ break;
+
+ case PS_2: /* Second label character */
+ if (*p)
+ {
+ label[1] = *(p++);
+ state = PS_3;
+ }
+ else
+ state = PS_FAIL; /* Error */
+ break;
+
+ case PS_3: /* Equal sign after indicator label */
+ state = PS_FAIL; /* Assume failure... */
+ if (*(p++) == '=')/* It *should* be... */
+ {
+ for (ind_no = 0; indicator_name[ind_no] != NULL; ++ind_no)
+ {
+ if (STREQ (label, indicator_name[ind_no]))
+ {
+ color_indicator[ind_no].string = buf;
+ state = (get_funky_string (&buf, &p, false,
+ &color_indicator[ind_no].len)
+ ? PS_START : PS_FAIL);
+ break;
+ }
+ }
+ if (state == PS_FAIL)
+ error (0, 0, _("unrecognized prefix: %s"), label);
+ }
+ break;
+
+ case PS_4: /* Equal sign after *.ext */
+ if (*(p++) == '=')
+ {
+ ext->seq.string = buf;
+ state = (get_funky_string (&buf, &p, false, &ext->seq.len)
+ ? PS_START : PS_FAIL);
+ }
+ else
+ state = PS_FAIL;
+ break;
+
+ case PS_FAIL:
+ goto done;
+
+ default:
+ abort ();
+ }
+ }
+ done:
+
+ if (state == PS_FAIL)
+ {
+ struct color_ext_type *e;
+ struct color_ext_type *e2;
+
+ error (0, 0,
+ _("unparsable value for DIFF_COLORS environment variable"));
+ free (color_buf);
+ for (e = color_ext_list; e != NULL; /* empty */)
+ {
+ e2 = e;
+ e = e->next;
+ free (e2);
+ }
+ colors_enabled = false;
+ }
+}
+
static void
check_color_output (bool is_pipe)
{
@@ -323,6 +713,9 @@ check_color_output (bool is_pipe)
colors_enabled = (colors_style == ALWAYS
|| (colors_style == AUTO && output_is_tty));
+ if (colors_enabled)
+ parse_diff_color ();
+
if (output_is_tty)
install_signal_handlers ();
}
@@ -923,12 +1316,27 @@ output_1_line (char const *base, char const *limit, char const *flag_format,
}
}
+enum indicator_no
+ {
+ C_LEFT, C_RIGHT, C_END, C_RESET, C_HEADER, C_ADD, C_DELETE, C_LINE
+ };
+
+static void
+put_indicator (const struct bin_str *ind)
+{
+ fwrite (ind->string, ind->len, 1, outfile);
+}
+
void
set_header_color_context (void)
{
process_signals ();
if (colors_enabled)
- fputs ("\x1B[1m", outfile);
+ {
+ put_indicator (&color_indicator[C_LEFT]);
+ put_indicator (&color_indicator[C_HEADER]);
+ put_indicator (&color_indicator[C_RIGHT]);
+ }
}
void
@@ -936,7 +1344,11 @@ set_line_numbers_color_context (void)
{
process_signals ();
if (colors_enabled)
- fputs ("\x1B[36m", outfile);
+ {
+ put_indicator (&color_indicator[C_LEFT]);
+ put_indicator (&color_indicator[C_LINE]);
+ put_indicator (&color_indicator[C_RIGHT]);
+ }
}
void
@@ -944,7 +1356,11 @@ set_add_color_context (void)
{
process_signals ();
if (colors_enabled)
- fputs ("\x1B[32m", outfile);
+ {
+ put_indicator (&color_indicator[C_LEFT]);
+ put_indicator (&color_indicator[C_ADD]);
+ put_indicator (&color_indicator[C_RIGHT]);
+ }
}
void
@@ -952,17 +1368,22 @@ set_delete_color_context (void)
{
process_signals ();
if (colors_enabled)
- fputs ("\x1B[31m", outfile);
+ {
+ put_indicator (&color_indicator[C_LEFT]);
+ put_indicator (&color_indicator[C_DELETE]);
+ put_indicator (&color_indicator[C_RIGHT]);
+ }
}
void
reset_color_context (void)
{
- static char const reset_sequence[] = "\x1b[0m";
- if (! colors_enabled)
- return;
-
- fputs (reset_sequence, outfile);
+ if (colors_enabled)
+ {
+ put_indicator (&color_indicator[C_LEFT]);
+ put_indicator (&color_indicator[C_RESET]);
+ put_indicator (&color_indicator[C_RIGHT]);
+ }
}
char const change_letter[] = { 0, 'd', 'a', 'c' };
--
2.5.0
>From 1e9f443fcd587bd6e137d7c627d3a250d09ca5c2 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <gscri...@redhat.com>
Date: Mon, 2 Nov 2015 19:03:32 +0000
Subject: [PATCH 2/3] doc: mention --color and --palette in NEWS
---
NEWS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/NEWS b/NEWS
index 7cdfedd..685dc9d 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,13 @@ GNU diffutils NEWS -*- outline -*-
* Noteworthy changes in release ?.? (????-??-??) [?]
+** New features
+
+ diff accepts two new options --color and --palette to set a colored
+ output. --color takes an optional argument specifying when to
+ colorize a line: --color=always, --color=auto, --color=never.
+ --palette is used to change the used colors.
+
** Bug fixes
When binary files differ, diff now exits with status 1 as POSIX requires.
--
2.5.0
>From 091ffa10f77e0e006d7173b2e7efb21966075824 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <gscri...@redhat.com>
Date: Mon, 2 Nov 2015 19:05:10 +0000
Subject: [PATCH 3/3] tests: Add tests for --color and --palette
* tests/colors: New file.
* tests/Makefile.am (TESTS): Add colors.
---
tests/Makefile.am | 3 ++-
tests/colors | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+), 1 deletion(-)
create mode 100755 tests/colors
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 438fbdf..805ccc2 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -15,7 +15,8 @@ TESTS = \
no-newline-at-eof \
stdin \
strcoll-0-names \
- filename-quoting
+ filename-quoting \
+ colors
EXTRA_DIST = \
$(TESTS) init.sh t-local.sh
diff --git a/tests/colors b/tests/colors
new file mode 100755
index 0000000..9e1ef40
--- /dev/null
+++ b/tests/colors
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+
+TZ=UTC0
+export TZ
+
+fail=0
+
+echo a > a
+echo b > b
+
+touch --date='1970-01-01 00:00:00' a b
+
+# Compare with some known outputs
+
+diff --color=always a b | sha1sum \
+ | grep dbed959c9975cf761ff4950d93d342d7c271c11f || fail=1
+
+diff --color=auto a b | sha1sum \
+ | grep 90742ce0d628cc2f2067b232404578e000b80cce || fail=1
+
+diff --color=never a b | sha1sum \
+ | grep 90742ce0d628cc2f2067b232404578e000b80cce || fail=1
+
+diff --color a b | sha1sum \
+ | grep 90742ce0d628cc2f2067b232404578e000b80cce || fail=1
+
+diff -u --color=always a b | sha1sum \
+ | grep 5712fbffc94c501eaeec5cb02468ce2bbed6d7c9 || fail=1
+
+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
+
+Exit $fail
--
2.5.0
