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

[Giuseppe Scrivano]

Paul Eggert <address@hidden> writes:

> On 03/12/2015 02:18 PM, Giuseppe Scrivano wrote:
>
>> +mbiter
>
> Ouch.  I was hoping we didn't need to do that.  I expect the use of
> mbiter to hurt performance significantly.  I hope there's a better
> way, one that doesn't require counting individual multibyte
> characters.  It may be time to think about having a signal handler
> that resets the output tty.

Thanks once again for your prompt and detailed review.

I have dropped that and implemented the solution you suggested: now the
signal handler directly write(2) to reset the tty if needed and then
restore the default handler.  It contributes to simplify the code in
util.c as the output throttling is not required anymore.

Giuseppe

>From 45372bec9789e0ec3879c8710dc0e4c799dd0950 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <address@hidden>
Date: Sun, 8 Mar 2015 22:45:11 +0100
Subject: [PATCH] diff: add support for --color

* bootstrap.conf (gnulib_modules): Add "sigprocmask".
* doc/diffutils.texi (diff Options): Add documentation for --color.
Copied from coreutils ls --color.
* src/context.c (pr_unidiff_hunk): Add calls to set_color_context.
* src/diff.h (enum colors): New enum to register the current color to use.
(enum colors_style): New enum to record when to use colors.
(colors_style): New variable to memorize the argument value.
(set_color_context): Add function definition.
* src/diff.c: : Define COLOR_OPTION.
(specify_colors_style): New function.
(longopts): Add --color.
(main): Call install_signal_handlers.  Handle --color argument.
(option_help_msgid): Add usage string for --color.
* src/normal.c (print_normal_hunk): Add calls to set_color_context
* src/side.c (print_1sdiff_line): Add calls to set_color_context.
* src/util.c (colors_enabled): New boolean variable.
(colors_enabled): New boolean variable.
(check_color_output): New function.
(install_signal_handlers): New function.
(signal_handler): New function.
(begin_output): Call check_color_output every time the output file is changed.
(set_color_context): New function.  If colors are enabled, print the right
command for the terminal to change the color.
---
 bootstrap.conf     |   1 +
 doc/diffutils.texi |  20 +++++++++++
 src/context.c      |   4 +++
 src/diff.c         |  32 ++++++++++++++++-
 src/diff.h         |  31 +++++++++++++++++
 src/normal.c       |  20 ++++++++---
 src/side.c         |  15 ++++++++
 src/util.c         | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 218 insertions(+), 5 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index 9b2de22..63be732 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -61,6 +61,7 @@ readme-release
 regex
 sh-quote
 signal
+sigprocmask
 stat
 stat-macros
 stat-time
diff --git a/doc/diffutils.texi b/doc/diffutils.texi
index 3e25807..0a2f1fc 100644
--- a/doc/diffutils.texi
+++ b/doc/diffutils.texi
@@ -3745,6 +3745,26 @@ Read and write data in binary mode.  @xref{Binary}.
 Use the context output format, showing three lines of context.
 @xref{Context Format}.
 
address@hidden --color address@hidden
address@hidden color, distinguishing file types with
+Specify whether to use color for distinguishing file types.  @var{when}
+may be omitted, or one of:
address@hidden @bullet
address@hidden none
address@hidden none @r{color option}
+Do not use color at all.  This is the default when no --color option
+is present.
address@hidden auto
address@hidden auto @r{color option}
address@hidden terminal, using color iff
+Only use color if standard output is a terminal.
address@hidden always
address@hidden always @r{color option}
+Always use color.
address@hidden itemize
+Specifying @option{--color} and no @var{when} is equivalent to
address@hidden
+
 @item -C @var{lines}
 @itemx address@hidden@address@hidden
 Use the context output format, showing @var{lines} (an integer) lines of
diff --git a/src/context.c b/src/context.c
index e0f21c4..f07a581 100644
--- a/src/context.c
+++ b/src/context.c
@@ -366,10 +366,12 @@ pr_unidiff_hunk (struct change *hunk)
          while (k--)
            {
              char const * const *line = &files[0].linbuf[i++];
+             set_color_context (DELETE, false);
              putc ('-', out);
              if (initial_tab && ! (suppress_blank_empty && **line == '\n'))
                putc ('\t', out);
              print_1_line (NULL, line);
+             set_color_context (RESET, false);
            }
 
          /* Then output the inserted part. */
@@ -378,10 +380,12 @@ pr_unidiff_hunk (struct change *hunk)
          while (k--)
            {
              char const * const *line = &files[1].linbuf[j++];
+             set_color_context (ADD, false);
              putc ('+', out);
              if (initial_tab && ! (suppress_blank_empty && **line == '\n'))
                putc ('\t', out);
              print_1_line (NULL, line);
+             set_color_context (RESET, false);
            }
 
          /* We're done with this hunk, so on to the next! */
diff --git a/src/diff.c b/src/diff.c
index ff28377..637f716 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -70,6 +70,7 @@ static void add_regexp (struct regexp_list *, char const *);
 static void summarize_regexp_list (struct regexp_list *);
 static void specify_style (enum output_style);
 static void specify_value (char const **, char const *, char const *);
+static void specify_colors_style (char const *);
 static void try_help (char const *, char const *) __attribute__((noreturn));
 static void check_stdout (void);
 static void usage (void);
@@ -136,7 +137,9 @@ enum
   UNCHANGED_GROUP_FORMAT_OPTION,
   OLD_GROUP_FORMAT_OPTION,
   NEW_GROUP_FORMAT_OPTION,
-  CHANGED_GROUP_FORMAT_OPTION
+  CHANGED_GROUP_FORMAT_OPTION,
+
+  COLOR_OPTION,
 };
 
 static char const group_format_option[][sizeof "--unchanged-group-format"] =
@@ -159,6 +162,7 @@ static struct option const longopts[] =
   {"binary", 0, 0, BINARY_OPTION},
   {"brief", 0, 0, 'q'},
   {"changed-group-format", 1, 0, CHANGED_GROUP_FORMAT_OPTION},
+  {"color", 2, 0, COLOR_OPTION},
   {"context", 2, 0, 'C'},
   {"ed", 0, 0, 'e'},
   {"exclude", 1, 0, 'x'},
@@ -285,6 +289,8 @@ main (int argc, char **argv)
   re_set_syntax (RE_SYNTAX_GREP | RE_NO_POSIX_BACKTRACKING);
   excluded = new_exclude ();
 
+  install_signal_handlers ();
+
   /* Decode the options.  */
 
   while ((c = getopt_long (argc, argv, shortopts, longopts, NULL)) != -1)
@@ -627,6 +633,10 @@ main (int argc, char **argv)
          specify_value (&group_format[c], optarg, group_format_option[c]);
          break;
 
+        case COLOR_OPTION:
+          specify_colors_style (optarg);
+         break;
+
        default:
          try_help (NULL, NULL);
        }
@@ -645,6 +655,9 @@ main (int argc, char **argv)
        specify_style (OUTPUT_NORMAL);
     }
 
+  if (colors_style != NEVER && paginate)
+    error (EXIT_TROUBLE, 0, _("cannot specify both --color and --paginate"));
+
   if (output_style != OUTPUT_CONTEXT || hard_locale (LC_TIME))
     {
 #if (defined STAT_TIMESPEC || defined STAT_TIMESPEC_NS \
@@ -940,6 +953,8 @@ static char const * const option_help_msgid[] = {
   N_("-d, --minimal            try hard to find a smaller set of changes"),
   N_("    --horizon-lines=NUM  keep NUM lines of the common prefix and 
suffix"),
   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_("    --help               display this help and exit"),
   N_("-v, --version            output version information and exit"),
@@ -1008,6 +1023,21 @@ specify_style (enum output_style style)
       output_style = style;
     }
 }
+
+/* Set the color mode.  */
+static void
+specify_colors_style (char const *value)
+{
+  if (value == NULL || STREQ (value, "auto"))
+    colors_style = AUTO;
+  else if (STREQ (value, "always"))
+    colors_style = ALWAYS;
+  else if (STREQ (value, "never"))
+    colors_style = NEVER;
+  else
+    try_help ("invalid color '%s'", value);
+}
+
 
 /* Set the last-modified time of *ST to be the current time.  */
 
diff --git a/src/diff.h b/src/diff.h
index 465e4bc..c18786e 100644
--- a/src/diff.h
+++ b/src/diff.h
@@ -38,6 +38,32 @@ enum changes
   /* Both deletes and inserts: a hunk containing both old and new lines.  */
   CHANGED
 };
+
+/* What kind of changes a hunk contains.  */
+enum colors
+{
+  /* Reset to the default color.  */
+  RESET,
+
+  /* Delete lines.  Show output in red.  */
+  DELETE,
+
+  /* Added lines.  Show them in green.  */
+  ADD,
+};
+
+/* What kind of changes a hunk contains.  */
+enum colors_style
+{
+  /* Never output colors.  */
+  NEVER,
+
+  /* Output colors if the output is a terminal.  */
+  AUTO,
+
+  /* Always output colors.  */
+  ALWAYS,
+};
 
 /* Variables for command line options */
 
@@ -83,6 +109,9 @@ enum output_style
 
 XTERN enum output_style output_style;
 
+/* True if colors are printed.  */
+XTERN enum colors_style colors_style;
+
 /* Nonzero if output cannot be generated for identical files.  */
 XTERN bool no_diff_means_no_output;
 
@@ -390,3 +419,5 @@ extern void print_script (struct change *, struct change * 
(*) (struct change *)
 extern void setup_output (char const *, char const *, bool);
 extern void translate_range (struct file_data const *, lin, lin,
                              long int *, long int *);
+extern void set_color_context (enum colors, bool force);
+extern void install_signal_handlers (void);
diff --git a/src/normal.c b/src/normal.c
index 721fd1a..41a97b9 100644
--- a/src/normal.c
+++ b/src/normal.c
@@ -56,14 +56,26 @@ print_normal_hunk (struct change *hunk)
 
   /* Print the lines that the first file has.  */
   if (changes & OLD)
-    for (i = first0; i <= last0; i++)
-      print_1_line ("<", &files[0].linbuf[i]);
+    {
+      for (i = first0; i <= last0; i++)
+        {
+          set_color_context (DELETE, false);
+          print_1_line ("<", &files[0].linbuf[i]);
+          set_color_context (RESET, false);
+        }
+    }
 
   if (changes == CHANGED)
     fputs ("---\n", outfile);
 
   /* Print the lines that the second file has.  */
   if (changes & NEW)
-    for (i = first1; i <= last1; i++)
-      print_1_line (">", &files[1].linbuf[i]);
+    {
+      for (i = first1; i <= last1; i++)
+        {
+          set_color_context (ADD, false);
+          print_1_line (">", &files[1].linbuf[i]);
+          set_color_context (RESET, false);
+        }
+    }
 }
diff --git a/src/side.c b/src/side.c
index 155512c..4d04942 100644
--- a/src/side.c
+++ b/src/side.c
@@ -206,6 +206,18 @@ print_1sdiff_line (char const *const *left, char sep,
   size_t c2o = sdiff_column2_offset;
   size_t col = 0;
   bool put_newline = false;
+  bool color_to_reset = false;
+
+  if (sep == '<')
+    {
+      set_color_context (DELETE, false);
+      color_to_reset = true;
+    }
+  else if (sep == '>')
+    {
+      set_color_context (ADD, false);
+      color_to_reset = true;
+    }
 
   if (left)
     {
@@ -233,6 +245,9 @@ print_1sdiff_line (char const *const *left, char sep,
 
   if (put_newline)
     putc ('\n', out);
+
+  if (color_to_reset)
+    set_color_context (RESET, false);
 }
 
 /* Print lines common to both files in side-by-side format.  */
diff --git a/src/util.c b/src/util.c
index 2d6d3fc..4cbd402 100644
--- a/src/util.c
+++ b/src/util.c
@@ -24,6 +24,7 @@
 #include <system-quote.h>
 #include <xalloc.h>
 #include "xvasprintf.h"
+#include <signal.h>
 
 char const pr_program[] = PR_PROGRAM;
 
@@ -153,6 +154,21 @@ print_message_queue (void)
 static char const *current_name0;
 static char const *current_name1;
 static bool currently_recursive;
+static bool colors_enabled;
+static bool output_is_tty;
+
+static void
+check_color_output (bool is_pipe)
+{
+  if (! outfile)
+    return;
+
+  output_is_tty = (colors_style != NEVER && !is_pipe
+                   && isatty (fileno (outfile)));
+
+  colors_enabled = (colors_style == ALWAYS
+                    || (colors_style == AUTO && output_is_tty));
+}
 
 void
 setup_output (char const *name0, char const *name1, bool recursive)
@@ -313,6 +329,7 @@ begin_output (void)
            outfile = fdopen (pipes[1], "w");
            if (!outfile)
              pfatal_with_name ("fdopen");
+           check_color_output (true);
          }
 #else
        char *command = system_quote_argv (SCI_SYSTEM, (char **) argv);
@@ -320,6 +337,7 @@ begin_output (void)
        outfile = popen (command, "w");
        if (!outfile)
          pfatal_with_name (command);
+       check_color_output (true);
        free (command);
 #endif
       }
@@ -330,6 +348,7 @@ begin_output (void)
       /* If -l was not specified, output the diff straight to 'stdout'.  */
 
       outfile = stdout;
+      check_color_output (false);
 
       /* If handling multiple files (because scanning a directory),
         print which files the following output is about.  */
@@ -717,6 +736,87 @@ output_1_line (char const *base, char const *limit, char 
const *flag_format,
     }
 }
 
+static void
+signal_handler (int signal)
+{
+  struct sigaction act;
+
+  if (output_is_tty)
+    set_color_context (RESET, true);
+
+  /* Restore the default handler, and report the signal again.  */
+  sigaction (signal, NULL, &act);
+  act.sa_handler = SIG_DFL;
+  sigaction (signal, &act, NULL);
+  raise (signal);
+}
+
+void
+install_signal_handlers (void)
+{
+  int j;
+  struct sigaction act;
+  int const sig[] =
+    {
+      SIGTSTP,
+      SIGALRM, SIGHUP, SIGINT, SIGPIPE, SIGQUIT, SIGTERM,
+#ifdef SIGPOLL
+      SIGPOLL,
+#endif
+#ifdef SIGPROF
+      SIGPROF,
+#endif
+#ifdef SIGVTALRM
+      SIGVTALRM,
+#endif
+#ifdef SIGXCPU
+      SIGXCPU,
+#endif
+#ifdef SIGXFSZ
+      SIGXFSZ,
+#endif
+    };
+
+  for (j = 0; j < sizeof (sig) / sizeof (*sig); j++)
+    {
+      sigaction (sig[j], NULL, &act);
+      if (act.sa_handler != SIG_IGN)
+        {
+          act.sa_handler = signal_handler;
+          sigaction (sig[j], &act, NULL);
+        }
+    }
+}
+
+void
+set_color_context (enum colors con, bool force)
+{
+  const char *const reset_sequence = "\x1b[0m";
+  if (! colors_enabled)
+    return;
+  switch (con)
+    {
+    case DELETE:
+      fprintf (outfile, "\x1B[31m");
+      break;
+
+    case ADD:
+      fprintf (outfile, "\x1B[32m");
+      break;
+
+    case RESET:
+      if (! force)
+        fprintf (outfile, "%s", reset_sequence);
+      else
+        {
+          if (write (fileno (outfile), reset_sequence,
+                     strlen (reset_sequence)) < 0)
+            error (EXIT_TROUBLE, 0, "%s", _("write failed"));
+        }
+      break;
+    }
+}
+
 char const change_letter[] = { 0, 'd', 'a', 'c' };
 
 /* Translate an internal line number (an index into diff's table of lines)
-- 
2.1.0