[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --co
From: |
Jim Meyering |
Subject: |
[bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color |
Date: |
Thu, 26 Nov 2015 09:17:24 -0800 |
On Mon, Nov 16, 2015 at 4:19 PM, Giuseppe Scrivano <address@hidden> wrote:
> Jim Meyering <address@hidden> writes:
>
>> On Tue, Nov 3, 2015 at 9:27 AM, Eric Blake <address@hidden> 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 address@hidden@address@hidden
>>>> 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}.
>>>>
>>>> address@hidden address@hidden
>>>> +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
- [bug-diffutils] bug#20062: bug#20062: bug#20062: bug#20062: bug#20062: bug#20062: bug#20062: [PATCH] diff: add support for --color, (continued)
- [bug-diffutils] bug#20062: bug#20062: bug#20062: bug#20062: bug#20062: bug#20062: bug#20062: [PATCH] diff: add support for --color, Jim Meyering, 2015/11/02
- [bug-diffutils] bug#20062: bug#20062: bug#20062: bug#20062: bug#20062: bug#20062: bug#20062: [PATCH] diff: add support for --color, Giuseppe Scrivano, 2015/11/02
- [bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color, Giuseppe Scrivano, 2015/11/03
- [bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color, Eric Blake, 2015/11/03
- [bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color, Jim Meyering, 2015/11/03
- [bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color, Giuseppe Scrivano, 2015/11/16
- [bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color,
Jim Meyering <=
- [bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color, Giuseppe Scrivano, 2015/11/27
- [bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color, Jim Meyering, 2015/11/27
- [bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color, Giuseppe Scrivano, 2015/11/28
- [bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color, Jim Meyering, 2015/11/28
- [bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color, Giuseppe Scrivano, 2015/11/29
- [bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color, Jim Meyering, 2015/11/29
- [bug-diffutils] bug#20062: bug#20062: bug#20062: [PATCH] diff: add support for --color, Eric Blake, 2015/11/03