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

Previous Next

Full log

View this message in rfc822 format

From: Jim Meyering <jim <at> meyering.net>
To: Giuseppe Scrivano <gscrivano <at> gnu.org>
Cc: Gisle Vanem <gvanem <at> yahoo.no>, 22067 <at> debbugs.gnu.org
Subject: bug#22067: [bug-diffutils] bug#22067: bug#22067: bug#20062: bug#20062: [PATCH] diff: add support for --color
Date: Thu, 11 Feb 2016 11:14:23 -0800

On Thu, Feb 11, 2016 at 4:43 AM, Giuseppe Scrivano <gscrivano <at> gnu.org> wrote:
> Jim Meyering <jim <at> meyering.net> writes:
>
>> On Wed, Feb 10, 2016 at 8:45 AM, Giuseppe Scrivano <gscrivano <at> gnu.org> wrote:
>>> Gisle Vanem <gvanem <at> yahoo.no> writes:
>>>
>>>> Giuseppe Scrivano wrote:
>>>>
>>>>> thanks for your patches.  Is it fine for you if we keep bug-diffutils in
>>>>> the loop?
>>>>
>>>> Sure. I forgot it was in the CC-list.
>>>> Attached again; wincolor.c + diff-1.txt.
>>>> Excused my diff format; I'm a "git n00b". Hope you figure it out.
>>>
>>> cannot talk for the diff maintainers, but personally I would split it
>>> two patches: one that fixes the build on Windows, I would not care to
>>> disable them as in any case the user specifies when to use them or not;
>>> and another patch that adds the support for colors on Windows.
>>>
>>> In this way the first one could be applied immediately and there is more
>>> time to review the second one which adds support for colors on Windows.
>>>
>>> Anyway, let's wait for the maintainers' opinion about it.  My previous
>>> patch is also waiting to be applied.
>>
>> Hi Giuseppe,
>> The only thing missing from your infloop-fixing patch is an
>> addition to the regression test suite. Can you contrive an
>> example that induces the infinite recursion? It's not an
>> absolute requirement in this case, but would be nice...
>
> I can write one,

Great! Thank you.

> but it will need a change in the code as well, since
> the signals are installed only when outputting to a tty:
...
> -  if (output_is_tty)
> +  if (output_is_tty || getenv ("DIFF_INSTALL_SIGNALS"))
>      install_signal_handlers ();

However, we try very hard to avoid making tools depend on
environment variable settings more than they already do,
so how about a hidden, three-hyphen option, say,
---presume-output-tty, analogous to rm's ---presume-input-tty?

Previous Next

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