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

[Paul Eggert]

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.

>            char const * const *line = &files[0].linbuf[i++];
> +              set_color_context (DELETE);

Please use the same indenting (if the original uses tabs, do that), so 
that it's easier to read the diffs.

> +    error (EXIT_TROUBLE, 0, _("Cannot specify both --color and 
--paginate."));

"cannot", not "Cannot", and no period at the end (it's not a sentence).

> +              fwrite (mbi_cur_ptr (iter), mbi_len, 1, outfile);
> +              written += mbi_len;

fwrite may write fewer than mbi_len characters.

> -        putc (' ', out);
> +                {
> +                  written += putc (' ', out);
> +                }

No need for {} here.  On the other hand, putc does not return 1-or-0 so 
this usage is problematic.

> +          written += fprintf (out, flag_format, line_flag);

fprintf might return -1.

> +        written += putc (c, out);
> ...
> +        written += putc (c, out);

Again, putc might return values other than 0 and 1.

> +  if (output_is_tty && !set_initialized)
> +    {
> +      sigemptyset (&set);
> +      for (j = 0; j < sizeof (sig) / sizeof (*sig); j++)
> +        sigaddset (&set, sig[j]);
> +      set_initialized = true;
> +    }

This stuff can be done once, by 'main', with no need for a
set_initialized var.

> +repeat:

Let's rephrase it without the goto; it'll be easier to understand that 
way, and just as efficient I expect.