[bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color

[Giuseppe Scrivano]

Jim Meyering <address@hidden> writes:

> On Mon, Nov 2, 2015 at 11:34 AM, Eric Blake <address@hidden> 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 <address@hidden>
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 address@hidden@address@hidden
 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}.
 
address@hidden address@hidden
+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 <address@hidden>
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 <address@hidden>
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