Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

From: Jeff King <peff@peff.net>
Date: Oct 13, 2017, 01:47 UTC
Message-ID: <20171013014721.d4vesqv4v5j7tmk2@sigill.intra.peff.net>
In-Reply-To: <xmqqinfjykm2.fsf@gitster.mtv.corp.google.com>

On Fri, Oct 13, 2017 at 09:09:09AM +0900, Junio C Hamano wrote:

Show 13 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > ... Also
> > as an aside, I think this patch means that:
> >
> >   git -c color.ui=always add -p
> >
> > is broken (as would a hypothetical "git --default-color=always add -p").
> > That's sufficiently insane that I'm not sure we should care about it.
> 
> Do you mean that "'-c color.ui=always' from the command line is
> passed down to the invocations of 'git' the 'add' command makes, and
> would break output from 'diff-index' that 'add -i' wants to parse"?

Yes, exactly.

Show 6 quoted lines

> With the breakage that motivated "downgrade only for on-disk" change
> in mind, I do think that is the right behaviour.  Those third-party
> scripts we broke knew how '-c color.ui=always' works and depended on
> it, and I consider that the command line configuration getting
> passed around as an integral part of 'how it works'.  "Fixing" it
> will break them again.

Yeah, agreed. We cannot know what the script is expecting, so without that we cannot win, short of turning off color.ui entirely for plumbing.

> Let's take it as a signal that tells us that the script writers know
> what they are doing and leave it as a longish rope they can play with.

OK. For the record, I'm not against scrapping this whole thing and trying to rollback to your "plumbing never looks at color.ui" proposal. It's quite late in the -rc cycle to do that, but there's nothing that says we can't bump the release date if that's what we need to do to get it right.

If we ship v2.15 with the "color.ui=always really means auto", I don't think we'd want to undo that. So if we ship with what's in -rc1 (plus this new hack on top) I think that would be fairly final.

-Peff