On Tue, Oct 03, 2017 at 02:15:15AM -0400, Jeff King wrote:
> The two reasonable paths forward to me are:
>
> 1. Do nothing. Putting "color.ui = always" in your on-disk config is a
> bad idea, and the right fix is to stop doing it.
>
> 2. Make "always" a synonym for "auto". This has the advantage over (1)
> that you can't shoot yourself in the foot with it (so the expanded
> foot-shooting capabilities of 4c7f1819b don't matter). The
> disadvantage is that you can no longer do:
>
> git -c color.ui=always foo >not-a-tty
>
> to ask for color in all sub-programs of "foo". I have no idea if
> anybody cares. I came up with that example in 4c7f1819b as the most
> plausible reason somebody might actually care about "always", but
> I've never used it myself.
>
> And there _is_ a way to get the same thing, which is:
>
> GIT_PAGER_IN_USE=1 git foo >not-a-tty
>
> I.e., stay in "auto" but make the claim that "auto" really ought to
> be showing color because the output is going to be consumed
> eventually by a human. While it looks a bit funny in my made-up
> example above (because the variables says "pager" but we're not
> actually piping to a pager directly), I think this is the most
> plausible reason that an actual program might want to override the
> auto-color decision.
I've got a series mostly done for option 2. It actually touches quite a
few of the test scripts, since we use "-c color.ui=always" to test
color output. Normally having to touch the test suite a lot gives me
pause that we're breaking something for real users, but I think in this
case the test suite is doing something that normal users simply don't.
A variant of 2 (let's call it 2a) is to convert "always" to "auto"
_only_ for on-disk config, allowing "-c" to still work. This dull the
test-suite pain, but there are still several test scripts which use
test_config and need updating. And it introduces a weird inconsistency
that I'd just as soon skip.
If we buy into the idea that "color.ui=always" is actually a useful
construct to put in your on-disk config (and I'm not sure that I do),
then our options are basically:
3. Go back to the earlier behavior, which i what Junio's 2-patch
revert series does. That has two issues though:
- after patch 2, for-each-ref still respects color.ui=always,
even though it's plumbing. This is maybe-OK because we don't
generate colors unless somebody puts %C() codes into their
format.
- plumbing like diff-tree still doesn't respect color.ui=never
4. Read color.ui everywhere, but downgrade "always" to "auto" in
plumbing commands (which would have to mark themselves as such by
setting a global). This again falls into the "inconsistent and hard
to explain" category, though I think it at least behaves as people
would generally want. Except for people who really do want:
git -c color.always=true foo >file
to produce color when "foo" is a script that uses diff plumbing
under the hood. I don't think that _can_ be solved, since from the
plumbing perspective this is the same as the buggy add--interactive
case.
I still lean towards option 2 (kill off "always"), but I'm trying to
consider the full range of options.
-Peff
