Hi Paul, Paul Eggert <egg...@cs.ucla.edu> 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'.
>From 2568e589ad38845ab8eaf895a98a84b94a29b72d Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano <gscriv...@gnu.org> 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}. +@item --color [=@var{when}] +@cindex 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: +@itemize @bullet +@item none +@vindex none @r{color option} +Do not use color at all. This is the default when no --color option +is present. +@item auto +@vindex auto @r{color option} +@cindex terminal, using color iff +Only use color if standard output is a terminal. +@item always +@vindex always @r{color option} +Always use color. +@end itemize +Specifying @option{--color} and no @var{when} is equivalent to +@option{--color=auto}. + @item -C @var{lines} @itemx --context@r{[}=@var{lines}@r{]} 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 ff28377..be6e6e3 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
Regards, Giuseppe