Hi Jim,
Jim Meyering <j...@meyering.net> 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 <gscri...@redhat.com>
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.
+@item --color-scheme=@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 -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
