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

From: Giuseppe Scrivano <[email-masked]>
Date: Wed, 11 Mar 2015 00:43:57 +0100

Paul Eggert <[email-masked]> writes:

> Giuseppe Scrivano wrote:
>
>> +  sigemptyset (&set);
>> +  for (j = 0; j < (sizeof (sig) / sizeof (*sig)); j++)
>> +    sigaddset (&set, sig[j]);
>
> Shouldn't the above part be done just once?  Also, it has redundant parentheses.
>
>> +      sigprocmask (SIG_BLOCK, &set, &old_sigproc_set);
>
> The sigprocmask module needs to be added to bootstrap.conf.
>
> Can the proposed implementation block signals for an unbounded amount
> of time? That would be bad.  ls.c attempts to avoid this problem, and
> 'diff' should too.

The change of color context is done per line (except in normal.c where
at the moment it is per hunk but I will change it to be per line) so the
most it can hang is the time needed to print a single line.

> Also, 'diff --color' needn't mess with signal handling unless isatty
> (fileno (outfile)).

Should this also be done if --color=always is used?

Thanks,
Giuseppe