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

Previous Next

Full log

View this message in rfc822 format

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Giuseppe Scrivano <gscrivan <at> redhat.com>
Cc: Eric Blake <eblake <at> redhat.com>, 20062 <at> debbugs.gnu.org
Subject: bug#20062: [bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color
Date: Thu, 12 Mar 2015 14:46:28 -0700

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.

Previous Next

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