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

On Tue, Oct 03, 2017 at 07:38:24PM +0900, Junio C Hamano wrote:

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.

Yes, I agree that the "everybody is colored by default" is the root of
the problem. And in that sense all of this discussion is band-aid fixes
on that.

At the same time, I have a suspicion that "even plumbing is color=auto
by default" may actually be _helping_ in many scripts. Because they to
use the plumbing to show output to the user, respecting the user's
normal color preferences. That's just a hunch, though.

I also think trying to revert that "default" patch (4c7f1819) may be a
pretty big pain at this point.

  Side note: Sorry, I noticed while writing this that I got my commit
  references mixed up earlier between 4c7f1819 (which turned the default
  to auto even for plumbing) and 136c8c8b, my recent change to parse
  color.ui in more places. When I said the problem was exacerbated/made
  worse by 4c7f1819, I really meant 136c8c8b. Hopefully that didn't
  confuse the discussion too much.

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.

Here's what I came up with on the "color.ui=always is nonsense that we
should not offer" front. The number of patches may be a little daunting,
but most of them are just removing cases of "git -c color.ui=always"
from the tests.

  [01/12]: test-terminal: set TERM=vt100
  [02/12]: t4015: prefer --color to -c color.diff=always
  [03/12]: t4015: use --color with --color-moved
  [04/12]: t3701: use test-terminal to collect color output
  [05/12]: t7508: use test_terminal for color output
  [06/12]: t7502: use diff.noprefix for --verbose test
  [07/12]: t6006: drop "always" color config tests
  [08/12]: t3203: drop "always" color test
  [09/12]: t7301: use test_terminal to check color
  [10/12]: t3205: use --color instead of color.branch=always
  [11/12]: provide --color option for all ref-filter users
  [12/12]: color: make "always" the same as "auto" in config

 Documentation/config.txt           | 35 ++++++++++++-------------
 Documentation/git-for-each-ref.txt |  5 ++++
 Documentation/git-tag.txt          |  5 ++++
 builtin/for-each-ref.c             |  1 +
 builtin/tag.c                      |  1 +
 color.c                            |  2 +-
 t/t3203-branch-output.sh           |  8 +-----
 t/t3205-branch-color.sh            |  5 ++--
 t/t3701-add-interactive.sh         | 18 +++++++++----
 t/t4015-diff-whitespace.sh         | 53 +++++++++++++++++++-------------------
 t/t4202-log.sh                     |  2 +-
 t/t6006-rev-list-format.sh         | 23 +++++------------
 t/t6300-for-each-ref.sh            |  7 +++--
 t/t7004-tag.sh                     |  6 ++---
 t/t7006-pager.sh                   |  6 ++---
 t/t7301-clean-interactive.sh       |  5 ++--
 t/t7502-commit.sh                  |  4 +--
 t/t7508-status.sh                  | 41 +++++++++++++++--------------
 t/test-terminal.perl               |  1 +
 19 files changed, 115 insertions(+), 113 deletions(-)

-Peff