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

[Giuseppe Scrivano]

Jim Meyering <address@hidden> writes:

> Great! Thank you.
>
>> but it will need a change in the code as well, since
>> the signals are installed only when outputting to a tty:
> ...
>> -  if (output_is_tty)
>> +  if (output_is_tty || getenv ("DIFF_INSTALL_SIGNALS"))
>>      install_signal_handlers ();
>
> However, we try very hard to avoid making tools depend on
> environment variable settings more than they already do,
> so how about a hidden, three-hyphen option, say,
> ---presume-output-tty, analogous to rm's ---presume-input-tty?

I have added a test that uses the new option ---presume-output-tty.

>From 4aea918d31454fdeaada5e2453bea3dfc2e25f8b Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <address@hidden>
Date: Mon, 1 Feb 2016 09:58:52 +0100
Subject: [PATCH] Fix an infinite recursion with --color

* src/diff.h: New extern variable `presume_output_tty'.
* src/diff.c: New enum PRESUME_OUTPUT_TTY_OPTION.
(group_format_option): Add '-presume-output-tty'.
(main): Handle PRESUME_OUTPUT_TTY_OPTION.
* src/util.c: New variable `presume_output_tty'.
(check_color_output): Handle presume_output_tty.
(set_color_context): Call process_signals only when color_context is
not RESET_CONTEXT.
* tests/colors: Check that diff doesn't crash when interrupted
in the middle of a color sequence.

Reported by Gisle Vanem in http://debbugs.gnu.org/22067
---
 src/diff.c   | 9 +++++++++
 src/diff.h   | 2 ++
 src/util.c   | 7 +++++--
 tests/colors | 7 +++++++
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/src/diff.c b/src/diff.c
index 3a566b8..9bc1d96 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -141,6 +141,8 @@ enum
 
   COLOR_OPTION,
   COLOR_PALETTE_OPTION,
+
+  PRESUME_OUTPUT_TTY_OPTION,
 };
 
 static char const group_format_option[][sizeof "--unchanged-group-format"] =
@@ -219,6 +221,9 @@ static struct option const longopts[] =
   {"unified", 2, 0, 'U'},
   {"version", 0, 0, 'v'},
   {"width", 1, 0, 'W'},
+
+  /* This is solely for testing.  Do not document.  */
+  {"-presume-output-tty", no_argument, NULL, PRESUME_OUTPUT_TTY_OPTION},
   {0, 0, 0, 0}
 };
 
@@ -641,6 +646,10 @@ main (int argc, char **argv)
          set_color_palette (optarg);
          break;
 
+        case PRESUME_OUTPUT_TTY_OPTION:
+          presume_output_tty = true;
+          break;
+
        default:
          try_help (NULL, NULL);
        }
diff --git a/src/diff.h b/src/diff.h
index e4c138c..0983e7c 100644
--- a/src/diff.h
+++ b/src/diff.h
@@ -417,5 +417,7 @@ enum color_context
   LINE_NUMBER_CONTEXT,
 };
 
+XTERN bool presume_output_tty;
+
 extern void set_color_context (enum color_context color_context);
 extern void set_color_palette (char const *palette);
diff --git a/src/util.c b/src/util.c
index bf9ed97..d7b8925 100644
--- a/src/util.c
+++ b/src/util.c
@@ -44,6 +44,8 @@
 
 char const pr_program[] = PR_PROGRAM;
 
+bool presume_output_tty;
+
 /* Queue up one-line messages to be printed at the end,
    when -l is specified.  Each message is recorded with a 'struct msg'.  */
 
@@ -710,7 +712,7 @@ check_color_output (bool is_pipe)
   if (! outfile || colors_style == NEVER)
     return;
 
-  output_is_tty = !is_pipe && isatty (fileno (outfile));
+  output_is_tty = presume_output_tty || (!is_pipe && isatty (fileno 
(outfile)));
 
   colors_enabled = (colors_style == ALWAYS
                     || (colors_style == AUTO && output_is_tty));
@@ -1349,7 +1351,8 @@ static enum color_context last_context = RESET_CONTEXT;
 void
 set_color_context (enum color_context color_context)
 {
-  process_signals ();
+  if (color_context != RESET_CONTEXT)
+    process_signals ();
   if (colors_enabled && last_context != color_context)
     {
       put_indicator (&color_indicator[C_LEFT]);
diff --git a/tests/colors b/tests/colors
index facfd8d..df395ec 100755
--- a/tests/colors
+++ b/tests/colors
@@ -116,4 +116,11 @@ test $? = 1 || fail=1
 gen_exp_u > exp || framework_failure_
 compare exp out || fail=1
 
+mkfifo fifo
+printf '%*s' 1000000 | tr ' ' a > a
+printf '%*s' 1000000 | tr ' ' b > b
+head -c 10 < fifo > /dev/null &
+diff --color=always ---presume-output-tty a b > fifo
+test $? = 141 || fail=1
+
 Exit $fail
-- 
2.5.0

Thanks,
Giuseppe