GNU bug report logs - #20062
[PATCH] diff: add support for --color

Previous Next

Full log

Message #176 received at 20062 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Giuseppe Scrivano <gscrivano <at> gnu.org>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, 20062 <at> debbugs.gnu.org
Subject: Re: [bug-diffutils] bug#20062: bug#20062: bug#20062: bug#20062:
 bug#20062: [PATCH] diff: add support for --color
Date: Sun, 18 Oct 2015 10:26:00 -0700

Thank you for that patch and for your patience.
I've skimmed through and so far have only a question and a request:

Regarding this section:

+void
+set_header_color_context (void)
+{
+  process_signals ();
+  if (colors_enabled)
+    fprintf (outfile, "\x1B[1;39m");
+}
+
+void
+set_line_numbers_color_context (void)
+{
+  process_signals ();
+  if (colors_enabled)
+    fprintf (outfile, "\x1B[36m");
+}
+
+void
+set_add_color_context (void)
+{
+  process_signals ();
+  if (colors_enabled)
+    fprintf (outfile, "\x1B[32m");
+  fflush (outfile);
+}
+
+void
+set_delete_color_context (void)
+{
+  process_signals ();
+  if (colors_enabled)
+    fprintf (outfile, "\x1B[31m");
+}
+
+void
+reset_color_context (void)
+{
+  static char const reset_sequence[] = "\x1b[0m";
+  if (! colors_enabled)
+    return;
+
+  fputs (reset_sequence, outfile);
+}

Why does set_add_color_context call fflush, yet the others do not?
Please use fputs rather than fprintf for those literal strings.
The former is often far more efficient.

Finally, should there be some way to specify different colors,
e.g., for those who use different-background-colored terminals,
or for the color blind?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.