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

[Giuseppe Scrivano]

Hi Jim,

Jim Meyering <address@hidden> writes:

> Regarding the support for configurable colors, as already mentioned
> here, we really do want to avoid adding support for any more
> environment variables.  Would you please adjust the second
> patch to take the settings from a command line argument instead?
> With that, it will be trivial to add an envvar-honoring wrapper script,
> function or alias, for those who desire that convenience.
>
> The most important reason not to provide this functionality via
> an envvar is that it would then allow that envvar setting to alter
> the behavior of any program that invokes diff, and that sort of
> interaction is often surprising and undesirable.
>
> Thanks again for contributing!

Thanks for your review.

Would something like this patch work?

I have attached the difference from the previous version to make the
review easier as few lines are changed.

I have included the full ChangeLog entries in the commit message so it
will be easier to squash the patch on top of the previous one.

Regards,
Giuseppe

>From 0ed880f2e6f76809563597cc698da4c15b19ba1d Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <address@hidden>
Date: Mon, 2 Nov 2015 15:43:23 +0100
Subject: [PATCH] squash! diff: honor env variable DIFF_COLORS

diff: add --color-scheme

doc/diffutils.texi: Add documentation for --color-scheme
src/diff.h (set_color_scheme): New prototype.
src/diff.c (set_color_scheme): New function.
(color_scheme): 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 |  7 ++++---
 src/diff.c         |  6 ++++++
 src/diff.h         |  1 +
 src/util.c         | 10 +++++++++-
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/doc/diffutils.texi b/doc/diffutils.texi
index 75ad0d7..9f2cdbb 100644
--- a/doc/diffutils.texi
+++ b/doc/diffutils.texi
@@ -3763,9 +3763,10 @@ Always use color.
 Specifying @option{--color} and no @var{when} is equivalent to
 @option{--color=auto}.
 
-The colors are defined by the environment variable @env{DIFF_COLORS}
-and default to @samp{rs=0:hd=1:ad=32:de=31:ln=36}
-for red added lines, green deleted lines, cyan line numbers, bold header.
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 -C @var{lines}
diff --git a/src/diff.c b/src/diff.c
index 4e0f602..55ed69d 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -140,6 +140,7 @@ enum
   CHANGED_GROUP_FORMAT_OPTION,
 
   COLOR_OPTION,
+  COLOR_SCHEME_OPTION,
 };
 
 static char const group_format_option[][sizeof "--unchanged-group-format"] =
@@ -163,6 +164,7 @@ static struct option const longopts[] =
   {"brief", 0, 0, 'q'},
   {"changed-group-format", 1, 0, CHANGED_GROUP_FORMAT_OPTION},
   {"color", 2, 0, COLOR_OPTION},
+  {"color-scheme", 1, 0, COLOR_SCHEME_OPTION},
   {"context", 2, 0, 'C'},
   {"ed", 0, 0, 'e'},
   {"exclude", 1, 0, 'x'},
@@ -635,6 +637,10 @@ main (int argc, char **argv)
          specify_colors_style (optarg);
          break;
 
+       case COLOR_SCHEME_OPTION:
+         set_color_scheme (optarg);
+         break;
+
        default:
          try_help (NULL, NULL);
        }
diff --git a/src/diff.h b/src/diff.h
index 472fa93..883119e 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_scheme (const char *scheme);
diff --git a/src/util.c b/src/util.c
index 8d27bc7..108b8e0 100644
--- a/src/util.c
+++ b/src/util.c
@@ -562,6 +562,14 @@ static const char *const indicator_name[]=
     "lc", "rc", "ec", "rs", "hd", "ad", "de", "ln", NULL
   };
 
+static const char *color_scheme;
+
+void
+set_color_scheme (const char *scheme)
+{
+  color_scheme = scheme;
+}
+
 static void
 parse_diff_color (void)
 {
@@ -572,7 +580,7 @@ parse_diff_color (void)
   char label[3];               /* Indicator label */
   struct color_ext_type *ext;  /* Extension we are working on */
 
-  if ((p = getenv ("DIFF_COLORS")) == NULL || *p == '\0')
+  if ((p = color_scheme) == NULL || *p == '\0')
     return;
 
   ext = NULL;
-- 
2.5.0