On Mon, Nov 16, 2015 at 4:19 PM, Giuseppe Scrivano <gscriv...@gnu.org> wrote:
> Jim Meyering <j...@meyering.net> writes:
>
>> On Tue, Nov 3, 2015 at 9:27 AM, Eric Blake <ebl...@redhat.com> wrote:
>>> On 11/03/2015 10:05 AM, Giuseppe Scrivano wrote:
>>>
>>>> I have attached the patches that implement --palette, the missing tests
>>>> and update the NEWS file.
>>>>
>>>
>>>> +++ b/doc/diffutils.texi
>>>> @@ -3763,6 +3763,7 @@ Always use color.
>>>> Specifying @option{--color} and no @var{when} is equivalent to
>>>> @option{--color=auto}.
>>>>
>>>> +
>>>> @item -C @var{lines}
>>>
>>> Spurious change?
>>>
>>>> @itemx --context@r{[}=@var{lines}@r{]}
>>>> Use the context output format, showing @var{lines} (an integer) lines of
>>>> @@ -3890,6 +3891,11 @@ if-then-else format. @xref{Line Formats}.
>>>> @itemx --show-c-function
>>>> Show which C function each change is in. @xref{C Function Headings}.
>>>>
>>>> +@item --palette=@var{scheme}
>>>> +It allows to specify what colors are used to colorize the output. It
>>>
>>> Passive voice. Would sound better as:
>>>
>>> Specify what color palette to use when colored output to use.
>>
>> Thanks for the quick review, Eric.
>> I'll wait for the next iteration.
>
> sorry for taking so long, I hope the attached version is fine.I have begun reviewing carefully. I adjusted NEWS. Here is the modified paragraph: ** New features diff accepts two new options --color and --palette to generate and configure colored output. --color takes an optional argument specifying when to colorize a line: --color=always, --color=auto, --color=never. --palette is used to configure which colors are used. I looked at the tests/colors script: we cannot/should not use sha1sum for two reasons. 1) it is a short cut; better to include the precise expected output in each case. Using this approach, if/when a test fails, there is no record of what the expected output was. 2) the "sha1sum" command is not universally available by that name. On BSD-based systems it is called "sha1". Thus, I began the conversion, and in so doing, I found some room for improvement: with the current patches I have, diff -u emits a pair of identical color-changing escape sequences before each "+"-prefixed line: $ diff -u --color=always a b|cat -A ^[[1;39m--- a^I1969-12-31 16:00:00.000000000 -0800$ +++ b^I1969-12-31 16:00:00.000000000 -0800$ ^[[0m^[[36m@@ -1 +1 @@^[[0m$ ^[[31m-a$ ^[[32m^[[32m+b$ ^[[0m Notice also how the final \e[0m is on the final line by itself, with no following newline. Please adjust so that it appears at the end of the final line instead. I confirmed that git-diff appears to do the same thing, but noted that git uses \e[m instead (no "0" part). Do you know of any pros/cons for one or the other? I've attached the beginnings of the adjusted tests/colors script that I used to discover these things. Can you finish the job of converting it to use "compare" rather than sha1sum?
colors.sh
Description: Bourne shell script
