[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug-diffutils] bug#20062: bug#20062: bug#20062: bug#20062: bug#20062: [
|
From: |
Giuseppe Scrivano |
|
Subject: |
[bug-diffutils] bug#20062: bug#20062: bug#20062: bug#20062: bug#20062: [PATCH] diff: add support for --color |
|
Date: |
Wed, 14 Oct 2015 11:33:42 +0200 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Giuseppe Scrivano <address@hidden> writes:
> Hi Paul,
>
> Paul Eggert <address@hidden> writes:
>
>>> Either we block signals or we catch them and process as ls does (calling
>>> 'process_signals' periodically) that problem will still be present.
>>
>> True. However, blocking signals means we'll need to periodically
>> fflush-unblock-block, which will slow us down; the whole point of
>> stdio is efficiency via buffering, after all. Doing it the 'ls' way
>> avoids this overhead, as we'll need to fflush only when a signal has
>> actually arrived, plus we avoid the syscall overhead of periodic
>> unblock-block.
>
> I have attached a new version of the patch where I've copied the signals
> handling code from coreutils and ensure that output_1_line won't block
> for too long without checking `process_signals'.
Any comments?
Thanks,
Giuseppe
>From 1c74f569083f2dcc7456f6c6fde93f5436dc0c90 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
* doc/diffutils.texi (diff Options): Add documentation for --color.
Copied from coreutils ls --color.
* src/context.c (pr_unidiff_hunk): Set the color context.
(print_context_header): Likewise.
(pr_context_hunk): Likewise.
* src/diff.h (enum colors_style): New enum to record when to use colors.
(colors_style): New variable to memorize the argument value.
(set_add_color_context): Add function definition.
(set_delete_color_context): Likewise.
(set_header_color_context): Likewise.
(set_line_numbers_color_context): Likewise.
(reset_color_context): Likewise.
* src/diff.c: : Define COLOR_OPTION.
(specify_colors_style): New function.
(longopts): Add --color.
(main): Handle --color argument.
(option_help_msgid): Add usage string for --color.
* src/normal.c (print_normal_hunk): Set the color context.
* src/side.c (print_1sdiff_line): Likewise.
* src/util.c (colors_enabled): New boolean variable.
(begin_output): Call check_color_output once the output file is
configured.
(output_1_line): Periodically call `process_signals'.
(caught_signals): New sigset_t.
(colors_enabled): New boolean variable.
(interrupt_signal): New sig_atomic_t.
(stop_signal_count): New sig_atomic_t.
(check_color_output): New function.
(install_signal_handlers): Likewise. Copied from coreutils ls.
(process_signals): Likewise. Copied from coreutils ls.
(reset_color_context): Likewise.
(set_add_color_context): Likewise.
(set_delete_color_context): Likewise.
(set_header_color_context): Likewise.
(set_line_numbers_color_context): Likewise.
(sighandler): Likewise. Copied from coreutils ls.
(stophandler): Likewise. Copied from coreutils ls.
---
doc/diffutils.texi | 21 ++++
src/context.c | 51 +++++++--
src/diff.c | 27 ++++-
src/diff.h | 21 ++++
src/normal.c | 18 +++-
src/side.c | 15 +++
src/util.c | 309 +++++++++++++++++++++++++++++++++++++++++++++++------
7 files changed, 418 insertions(+), 44 deletions(-)
diff --git a/doc/diffutils.texi b/doc/diffutils.texi
index 091257f..0944b44 100644
--- a/doc/diffutils.texi
+++ b/doc/diffutils.texi
@@ -3742,6 +3742,27 @@ 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 different context
+Specify whether to use color for distinguishing different contexts,
+like header, added or removed lines. @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..8e9a74f 100644
--- a/src/context.c
+++ b/src/context.c
@@ -80,6 +80,7 @@ print_context_label (char const *mark,
void
print_context_header (struct file_data inf[], char const *const *names, bool
unidiff)
{
+ set_header_color_context ();
if (unidiff)
{
print_context_label ("---", &inf[0], names[0], file_label[0]);
@@ -90,6 +91,7 @@ print_context_header (struct file_data inf[], char const
*const *names, bool uni
print_context_label ("***", &inf[0], names[0], file_label[0]);
print_context_label ("---", &inf[1], names[1], file_label[1]);
}
+ reset_color_context ();
}
/* Print an edit script in context format. */
@@ -215,6 +217,7 @@ pr_context_hunk (struct change *hunk)
for (i = first0; i <= last0; i++)
{
+ bool reset_context = false;
/* Skip past changes that apply (in file 0)
only to lines before line I. */
@@ -225,12 +228,18 @@ pr_context_hunk (struct change *hunk)
prefix = " ";
if (next && next->line0 <= i)
- /* The change NEXT covers this line.
- If lines were inserted here in file 1, this is "changed".
- Otherwise it is "deleted". */
- prefix = (next->inserted > 0 ? "!" : "-");
+ {
+ reset_context = true;
+ set_delete_color_context ();
+ /* The change NEXT covers this line.
+ If lines were inserted here in file 1, this is "changed".
+ Otherwise it is "deleted". */
+ prefix = (next->inserted > 0 ? "!" : "-");
+ }
print_1_line (prefix, &files[0].linbuf[i]);
+ if (reset_context)
+ reset_color_context ();
}
}
@@ -244,6 +253,7 @@ pr_context_hunk (struct change *hunk)
for (i = first1; i <= last1; i++)
{
+ bool reset_context = false;
/* Skip past changes that apply (in file 1)
only to lines before line I. */
@@ -254,12 +264,17 @@ pr_context_hunk (struct change *hunk)
prefix = " ";
if (next && next->line1 <= i)
- /* The change NEXT covers this line.
- If lines were deleted here in file 0, this is "changed".
- Otherwise it is "inserted". */
- prefix = (next->deleted > 0 ? "!" : "+");
-
+ {
+ reset_context = true;
+ set_add_color_context ();
+ /* The change NEXT covers this line.
+ If lines were deleted here in file 0, this is "changed".
+ Otherwise it is "inserted". */
+ prefix = (next->deleted > 0 ? "!" : "+");
+ }
print_1_line (prefix, &files[1].linbuf[i]);
+ if (reset_context)
+ reset_color_context ();
}
}
}
@@ -330,11 +345,13 @@ pr_unidiff_hunk (struct change *hunk)
begin_output ();
out = outfile;
+ set_line_numbers_color_context ();
fputs ("@@ -", out);
print_unidiff_number_range (&files[0], first0, last0);
fputs (" +", out);
print_unidiff_number_range (&files[1], first1, last1);
fputs (" @@", out);
+ reset_color_context ();
if (function)
print_context_function (out, function);
@@ -360,9 +377,17 @@ pr_unidiff_hunk (struct change *hunk)
}
else
{
+ bool reset_context = false;
+
/* For each difference, first output the deleted part. */
k = next->deleted;
+ if (k)
+ {
+ reset_context = true;
+ set_delete_color_context ();
+ }
+
while (k--)
{
char const * const *line = &files[0].linbuf[i++];
@@ -375,9 +400,15 @@ pr_unidiff_hunk (struct change *hunk)
/* Then output the inserted part. */
k = next->inserted;
+ if (k)
+ {
+ reset_context = true;
+ set_add_color_context ();
+ }
while (k--)
{
char const * const *line = &files[1].linbuf[j++];
+ set_add_color_context ();
putc ('+', out);
if (initial_tab && ! (suppress_blank_empty && **line == '\n'))
putc ('\t', out);
@@ -386,6 +417,8 @@ pr_unidiff_hunk (struct change *hunk)
/* We're done with this hunk, so on to the next! */
+ if (reset_context)
+ reset_color_context ();
next = next->link;
}
}
diff --git a/src/diff.c b/src/diff.c
index efd7e47..4e0f602 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'},
@@ -627,6 +631,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);
}
@@ -940,6 +948,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 +1018,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..472fa93 100644
--- a/src/diff.h
+++ b/src/diff.h
@@ -38,6 +38,19 @@ enum changes
/* Both deletes and inserts: a hunk containing both old and new lines. */
CHANGED
};
+
+/* 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 +96,9 @@ enum output_style
XTERN enum output_style output_style;
+/* Define the current color context used to print a line. */
+XTERN enum colors_style colors_style;
+
/* Nonzero if output cannot be generated for identical files. */
XTERN bool no_diff_means_no_output;
@@ -390,3 +406,8 @@ 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_header_color_context (void);
+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);
diff --git a/src/normal.c b/src/normal.c
index 721fd1a..227af10 100644
--- a/src/normal.c
+++ b/src/normal.c
@@ -49,21 +49,31 @@ print_normal_hunk (struct change *hunk)
begin_output ();
/* Print out the line number header for this hunk */
+ set_line_numbers_color_context ();
print_number_range (',', &files[0], first0, last0);
fputc (change_letter[changes], outfile);
print_number_range (',', &files[1], first1, last1);
fputc ('\n', outfile);
+ reset_color_context ();
/* Print the lines that the first file has. */
if (changes & OLD)
- for (i = first0; i <= last0; i++)
- print_1_line ("<", &files[0].linbuf[i]);
+ {
+ set_delete_color_context ();
+ for (i = first0; i <= last0; i++)
+ print_1_line ("<", &files[0].linbuf[i]);
+ reset_color_context ();
+ }
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]);
+ {
+ set_add_color_context ();
+ for (i = first1; i <= last1; i++)
+ print_1_line (">", &files[1].linbuf[i]);
+ reset_color_context ();
+ }
}
diff --git a/src/side.c b/src/side.c
index 155512c..b762d31 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_delete_color_context ();
+ color_to_reset = true;
+ }
+ else if (sep == '>')
+ {
+ set_add_color_context ();
+ 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)
+ reset_color_context ();
}
/* Print lines common to both files in side-by-side format. */
diff --git a/src/util.c b/src/util.c
index 2d6d3fc..b5b39a2 100644
--- a/src/util.c
+++ b/src/util.c
@@ -24,6 +24,22 @@
#include <system-quote.h>
#include <xalloc.h>
#include "xvasprintf.h"
+#include <signal.h>
+
+/* Use SA_NOCLDSTOP as a proxy for whether the sigaction machinery is
+ present. */
+#ifndef SA_NOCLDSTOP
+# define SA_NOCLDSTOP 0
+# define sigprocmask(How, Set, Oset) /* empty */
+# define sigset_t int
+# if ! HAVE_SIGINTERRUPT
+# define siginterrupt(sig, flag) /* empty */
+# endif
+#endif
+
+#ifndef SA_RESTART
+# define SA_RESTART 0
+#endif
char const pr_program[] = PR_PROGRAM;
@@ -143,6 +159,152 @@ print_message_queue (void)
}
}
+/* The set of signals that are caught. */
+
+static sigset_t caught_signals;
+
+/* If nonzero, the value of the pending fatal signal. */
+
+static sig_atomic_t volatile interrupt_signal;
+
+/* A count of the number of pending stop signals that have been received. */
+
+static sig_atomic_t volatile stop_signal_count;
+
+/* An ordinary signal was received; arrange for the program to exit. */
+
+static void
+sighandler (int sig)
+{
+ if (! SA_NOCLDSTOP)
+ signal (sig, SIG_IGN);
+ if (! interrupt_signal)
+ interrupt_signal = sig;
+}
+
+/* A SIGTSTP was received; arrange for the program to suspend itself. */
+
+static void
+stophandler (int sig)
+{
+ if (! SA_NOCLDSTOP)
+ signal (sig, stophandler);
+ if (! interrupt_signal)
+ stop_signal_count++;
+}
+/* Process any pending signals. If signals are caught, this function
+ should be called periodically. Ideally there should never be an
+ unbounded amount of time when signals are not being processed.
+ Signal handling can restore the default colors, so callers must
+ immediately change colors after invoking this function. */
+
+static void
+process_signals (void)
+{
+ while (interrupt_signal || stop_signal_count)
+ {
+ int sig;
+ int stops;
+ sigset_t oldset;
+
+ reset_color_context ();
+ fflush (stdout);
+
+ sigprocmask (SIG_BLOCK, &caught_signals, &oldset);
+
+ /* Reload interrupt_signal and stop_signal_count, in case a new
+ signal was handled before sigprocmask took effect. */
+ sig = interrupt_signal;
+ stops = stop_signal_count;
+
+ /* SIGTSTP is special, since the application can receive that signal
+ more than once. In this case, don't set the signal handler to the
+ default. Instead, just raise the uncatchable SIGSTOP. */
+ if (stops)
+ {
+ stop_signal_count = stops - 1;
+ sig = SIGSTOP;
+ }
+ else
+ signal (sig, SIG_DFL);
+
+ /* Exit or suspend the program. */
+ raise (sig);
+ sigprocmask (SIG_SETMASK, &oldset, NULL);
+
+ /* If execution reaches here, then the program has been
+ continued (after being suspended). */
+ }
+}
+
+static void
+install_signal_handlers (void)
+{
+ /* The signals that are trapped, and the number of such signals. */
+ static int const sig[] =
+ {
+ /* This one is handled specially. */
+ SIGTSTP,
+
+ /* The usual suspects. */
+ 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
+ };
+ enum { nsigs = sizeof (sig) / sizeof *(sig) };
+
+#if ! SA_NOCLDSTOP
+ bool caught_sig[nsigs];
+#endif
+ {
+ int j;
+#if SA_NOCLDSTOP
+ struct sigaction act;
+
+ sigemptyset (&caught_signals);
+ for (j = 0; j < nsigs; j++)
+ {
+ sigaction (sig[j], NULL, &act);
+ if (act.sa_handler != SIG_IGN)
+ sigaddset (&caught_signals, sig[j]);
+ }
+
+ act.sa_mask = caught_signals;
+ act.sa_flags = SA_RESTART;
+
+ for (j = 0; j < nsigs; j++)
+ if (sigismember (&caught_signals, sig[j]))
+ {
+ act.sa_handler = sig[j] == SIGTSTP ? stophandler : sighandler;
+ sigaction (sig[j], &act, NULL);
+ }
+#else
+ for (j = 0; j < nsigs; j++)
+ {
+ caught_sig[j] = (signal (sig[j], SIG_IGN) != SIG_IGN);
+ if (caught_sig[j])
+ {
+ signal (sig[j], sig[j] == SIGTSTP ? stophandler : sighandler);
+ siginterrupt (sig[j], 0);
+ }
+ }
+#endif
+ }
+}
+
/* Call before outputting the results of comparing files NAME0 and NAME1
to set up OUTFILE, the stdio stream for the output to go to.
@@ -153,6 +315,24 @@ print_message_queue (void)
static char const *current_name0;
static char const *current_name1;
static bool currently_recursive;
+static bool colors_enabled;
+
+static void
+check_color_output (bool is_pipe)
+{
+ bool output_is_tty;
+
+ if (! outfile || colors_style == NEVER)
+ return;
+
+ output_is_tty = !is_pipe && isatty (fileno (outfile));
+
+ colors_enabled = (colors_style == ALWAYS
+ || (colors_style == AUTO && output_is_tty));
+
+ if (output_is_tty)
+ install_signal_handlers ();
+}
void
setup_output (char const *name0, char const *name1, bool recursive)
@@ -313,6 +493,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 +501,7 @@ begin_output (void)
outfile = popen (command, "w");
if (!outfile)
pfatal_with_name (command);
+ check_color_output (true);
free (command);
#endif
}
@@ -330,6 +512,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. */
@@ -672,8 +855,21 @@ void
output_1_line (char const *base, char const *limit, char const *flag_format,
char const *line_flag)
{
+ const size_t MAX_CHUNK = 1024;
if (!expand_tabs)
- fwrite (base, sizeof (char), limit - base, outfile);
+ {
+ size_t left = limit - base;
+ while (left)
+ {
+ size_t to_write = MIN (left, MAX_CHUNK);
+ size_t written = fwrite (base, sizeof (char), to_write, outfile);
+ if (written < to_write)
+ return;
+ base += written;
+ left -= written;
+ process_signals ();
+ }
+ }
else
{
register FILE *out = outfile;
@@ -681,40 +877,93 @@ output_1_line (char const *base, char const *limit, char
const *flag_format,
register char const *t = base;
register size_t column = 0;
size_t tab_size = tabsize;
+ size_t counter_proc_signals = 0;
while (t < limit)
- switch ((c = *t++))
- {
- case '\t':
- {
- size_t spaces = tab_size - column % tab_size;
- column += spaces;
- do
- putc (' ', out);
- while (--spaces);
- }
- break;
+ {
+ counter_proc_signals++;
+ if (counter_proc_signals == MAX_CHUNK)
+ {
+ process_signals ();
+ counter_proc_signals = 0;
+ }
+
+ switch ((c = *t++))
+ {
+ case '\t':
+ {
+ size_t spaces = tab_size - column % tab_size;
+ column += spaces;
+ do
+ putc (' ', out);
+ while (--spaces);
+ }
+ break;
+
+ case '\r':
+ putc (c, out);
+ if (flag_format && t < limit && *t != '\n')
+ fprintf (out, flag_format, line_flag);
+ column = 0;
+ break;
+
+ case '\b':
+ if (column == 0)
+ continue;
+ column--;
+ putc (c, out);
+ break;
+
+ default:
+ column += isprint (c) != 0;
+ putc (c, out);
+ break;
+ }
+ }
+ }
+}
- case '\r':
- putc (c, out);
- if (flag_format && t < limit && *t != '\n')
- fprintf (out, flag_format, line_flag);
- column = 0;
- break;
+void
+set_header_color_context (void)
+{
+ process_signals ();
+ if (colors_enabled)
+ fprintf (outfile, "\x1B[1;39m");
+}
- case '\b':
- if (column == 0)
- continue;
- column--;
- putc (c, out);
- break;
+void
+set_line_numbers_color_context (void)
+{
+ process_signals ();
+ if (colors_enabled)
+ fprintf (outfile, "\x1B[36m");
+}
- default:
- column += isprint (c) != 0;
- putc (c, out);
- break;
- }
- }
+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);
}
char const change_letter[] = { 0, 'd', 'a', 'c' };
--
2.4.3
- [bug-diffutils] bug#20062: bug#20062: bug#20062: bug#20062: bug#20062: [PATCH] diff: add support for --color,
Giuseppe Scrivano <=