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

[Jim Meyering]

On Sun, Nov 1, 2015 at 9:06 PM, Jim Meyering <address@hidden> wrote:
> On Tue, Oct 20, 2015 at 9:23 AM, Giuseppe Scrivano <address@hidden> wrote:
>> Hi Jim,
>>
>> Jim Meyering <address@hidden> writes:
>>
>>> Thank you for that patch and for your patience.
>>
>> thank you very much for the review!
>>
>>> I've skimmed through and so far have only a question and a request:
> ...
>>> Why does set_add_color_context call fflush, yet the others do not?
>>> Please use fputs rather than fprintf for those literal strings.
>>> The former is often far more efficient.
>>
>> Yes, it shouldn't as well.  I have changed it.
>
> Thanks for the quick work.
> I'll try to be quicker, this time.
>
>>> Finally, should there be some way to specify different colors,
>>> e.g., for those who use different-background-colored terminals,
>>> or for the color blind?
>>
>> I have took more code from coreutils ls and diff honors DIFF_COLORS
>> now.  I added it in a separate patch to facilitate the review.  Probably
>> all the shared code should end up in a gnulib module, but it probably
>> needs a better API before it can happen.
>>
>> Changes in the first patch:
>>
>> 1) dropped fflush from set_add_color_context
>> 2) use fputs instead of fprintf (the second patch replaces it)
>> 3) change the code color of the header to the default color to match
>> the "git diff" output.
>
> Those revisions to your first patch look fine, and I will look over that
> one once more in the morning, then expect to push.

Your first patch adds a useful new feature, but includes
neither a NEWS addition nor any test. Would you please add those?