Re: Updated to v2.14.2 on macOS; git add --patch broken

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 3, 2017, 10:38 UTC
Message-ID: <xmqqpoa4h5y7.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171003091049.jb46pyi3obtwkzsa@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 21 quoted lines

> On Tue, Oct 03, 2017 at 05:56:53PM +0900, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > Note that I'm arguing that it's a foot-gun even without scripts in the
>> > picture at all. Forget about plumbing versus porcelain. If you set
>> > color.ui to "always", you're going to get unexpected and confusing
>> > results from time to time.
>> 
>> Really? 
>> 
>> I would think you would consistently get ANSI colored output in any
>> medium, even in files that you would later "cat" or "less -R" to
>> view.  Is that unexpected?  Those who set "always" (I am not among
>> them, of course) would expect that, I would think.
>
> Those cases might be expected. But color when piping to grep or sed are
> not. I guess you can lump those under "well, they should be using
> plumbing, of course" but I don't think that's very realistic. People do
> ad-hoc pipes in their shells all the time (well, I assume so; I
> certainly do).

That's an argument to say color.ui=always is not a useful thing to use and Git is wrong to offer such a nonsense option. I agree with both of them.

But it is a different matter that plumbing commands are now doing the "color" thing without being asked. Reading the configuration that is meant for human interaction adds insult to injury. I think the earlier "everybody is colored by default" that forgot to make sure the change does not affect plumbing was the main culprit, and we were merely lucky that thanks to the auto-detection of "auto" we did not break scripts.

Having said all that, unless we revert the entire series (and possibly other things that happened after the series was merged on 'master' that assumes that now default_config would read the color.ui thing), we won't be able to get back to the state before the "add -p" regression, it seems. As -rc freeze period for the next cycle is approaching fast, I wanted to make sure that we have a plan to address the regression (which does not have to solve what the reverted commit tried to solve). If you think we can get a workable code in 2.14.x and 'master' that essentially castrates "always" and makes it the same as "auto" in several days tops, then I'd prefer such a solution, as honoring "color.ui=always" does not feel all that important.