git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jonathan Nieder <jrnieder@gmail.com>,
	Toni Uebernickel <tuebernickel@gmail.com>,
	git@vger.kernel.org, Tsvi Mostovicz <ttmost@gmail.com>
Subject: [PATCH 12/12] color: make "always" the same as "auto" in config
Date: Tue, 3 Oct 2017 09:46:06 -0400	[thread overview]
Message-ID: <20171003134606.2r2653znqidt2mgx@sigill.intra.peff.net> (raw)
In-Reply-To: <20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net>

It can be handy to use `--color=always` (or it's synonym
`--color`) on the command-line to convince a command to
produce color even if it's stdout isn't going to the
terminal or a pager.

What's less clear is whether it makes sense to set config
variables like color.ui to `always`. For a one-shot like:

  git -c color.ui=always ...

it's potentially useful (especially if the command doesn't
directly support the `--color` option). But setting `always`
in your on-disk config is much muddier, as you may be
surprised when piped commands generate colors (and send them
to whatever is consuming the pipe downstream).

Some people have done this anyway, because:

  1. The documentation for color.ui makes it sound like
     using `always` is a good idea, when you almost
     certainly want `auto`.

  2. Traditionally not every command (and especially not
     plumbing) respected color.ui in the first place. So
     the confusion came up less frequently than it might
     have.

The situation changed in 136c8c8b8f (color: check color.ui
in git_default_config(), 2017-07-13), which negated point
(2): now scripts using only plumbing commands (like
add-interactive) are broken by this setting.

That commit was fixing real issues (e.g., by making
`color.ui=never` work, since `auto` is the default), so we
don't want to just revert it.  We could turn `always` into a
noop in plumbing commands, but that creates a hard-to-explain
inconsistency between the plumbing and other commands.

Instead, let's just turn `always` into `auto` for all config.
This does break the "one-shot" config shown above, but again,
we're probably better to have simple and consistent rules than
to try to special-case command-line config.

There is one place where `always` should retain its meaning:
on the command line, `--color=always` should continue to be
the same as `--color`, overriding any isatty checks. Since the
command-line parser also depends on git_config_colorbool(), we
can use the existence of the "var" string to deterine whether
we are serving the command-line or the config.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/config.txt   | 35 +++++++++++++++++------------------
 color.c                    |  2 +-
 t/t3701-add-interactive.sh | 10 ++++++++++
 3 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1ac0ae6adb..b53c994d0a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1058,10 +1058,10 @@ clean.requireForce::
 
 color.branch::
 	A boolean to enable/disable color in the output of
-	linkgit:git-branch[1]. May be set to `always`,
-	`false` (or `never`) or `auto` (or `true`), in which case colors are used
-	only when the output is to a terminal. If unset, then the
-	value of `color.ui` is used (`auto` by default).
+	linkgit:git-branch[1]. May be set to `false` (or `never`) to
+	disable color entirely, `auto` (or `true` or `always`) in which
+	case colors are used only when the output is to a terminal.  If
+	unset, then the value of `color.ui` is used (`auto` by default).
 
 color.branch.<slot>::
 	Use customized color for branch coloration. `<slot>` is one of
@@ -1072,12 +1072,11 @@ color.branch.<slot>::
 
 color.diff::
 	Whether to use ANSI escape sequences to add color to patches.
-	If this is set to `always`, linkgit:git-diff[1],
+	If this is set to `true` or `auto`, linkgit:git-diff[1],
 	linkgit:git-log[1], and linkgit:git-show[1] will use color
-	for all patches.  If it is set to `true` or `auto`, those
-	commands will only use color when output is to the terminal.
-	If unset, then the value of `color.ui` is used (`auto` by
-	default).
+	when output is to the terminal. The value `always` is a
+	historical synonym for `auto`.  If unset, then the value of
+	`color.ui` is used (`auto` by default).
 +
 This does not affect linkgit:git-format-patch[1] or the
 'git-diff-{asterisk}' plumbing commands.  Can be overridden on the
@@ -1141,12 +1140,12 @@ color.grep.<slot>::
 --
 
 color.interactive::
-	When set to `always`, always use colors for interactive prompts
+	When set to `true` or `auto`, use colors for interactive prompts
 	and displays (such as those used by "git-add --interactive" and
-	"git-clean --interactive"). When false (or `never`), never.
-	When set to `true` or `auto`, use colors only when the output is
-	to the terminal. If unset, then the value of `color.ui` is
-	used (`auto` by default).
+	"git-clean --interactive") when the output is to the terminal.
+	When false (or `never`), never show colors. The value `always`
+	is a historical synonym for `auto`.  If unset, then the value of
+	`color.ui` is used (`auto` by default).
 
 color.interactive.<slot>::
 	Use customized color for 'git add --interactive' and 'git clean
@@ -1193,10 +1192,10 @@ color.ui::
 	configuration to set a default for the `--color` option.  Set it
 	to `false` or `never` if you prefer Git commands not to use
 	color unless enabled explicitly with some other configuration
-	or the `--color` option. Set it to `always` if you want all
-	output not intended for machine consumption to use color, to
-	`true` or `auto` (this is the default since Git 1.8.4) if you
-	want such output to use color when written to the terminal.
+	or the `--color` option. Set it to `true` or `auto` to enable
+	color when output is written to the terminal (this is also the
+	default since Git 1.8.4). The value `always` is a historical
+	synonym for `auto`.
 
 column.ui::
 	Specify whether supported commands should output in columns.
diff --git a/color.c b/color.c
index 9ccd954d6b..9c0dc82370 100644
--- a/color.c
+++ b/color.c
@@ -308,7 +308,7 @@ int git_config_colorbool(const char *var, const char *value)
 		if (!strcasecmp(value, "never"))
 			return 0;
 		if (!strcasecmp(value, "always"))
-			return 1;
+			return var ? GIT_COLOR_AUTO : 1;
 		if (!strcasecmp(value, "auto"))
 			return GIT_COLOR_AUTO;
 	}
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 39d0130f88..a49c12c79b 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -483,4 +483,14 @@ test_expect_success 'hunk-editing handles custom comment char' '
 	git diff --exit-code
 '
 
+test_expect_success 'add -p works even with color.ui=always' '
+	git reset --hard &&
+	echo change >>file &&
+	test_config color.ui always &&
+	echo y | git add -p &&
+	echo file >expect &&
+	git diff --cached --name-only >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.14.2.1079.gce6b466188

  parent reply	other threads:[~2017-10-03 13:46 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-27 13:23 Updated to v2.14.2 on macOS; git add --patch broken Toni Uebernickel
2017-09-27 17:07 ` Jeff King
2017-09-27 17:28   ` Toni Uebernickel
2017-09-27 18:01     ` Jeff King
2017-09-27 19:51       ` Jonathan Nieder
2017-09-27 19:53         ` Jonathan Nieder
2017-09-28  5:03           ` Toni Uebernickel
2017-09-28  5:20             ` Jeff King
2017-09-28  5:31               ` Toni Uebernickel
2017-10-02 23:00 ` Jonathan Nieder
2017-10-03  1:18   ` Junio C Hamano
2017-10-03  2:25     ` Junio C Hamano
2017-10-03  3:14       ` Junio C Hamano
2017-10-03  6:15       ` Jeff King
2017-10-03  7:10         ` Junio C Hamano
2017-10-03  7:16           ` Jeff King
2017-10-03  8:34             ` Junio C Hamano
2017-10-03  8:45               ` Jeff King
2017-10-03  8:56                 ` Junio C Hamano
2017-10-03  9:10                   ` Jeff King
2017-10-03  9:18                     ` Tsvi Mostovicz
2017-10-03 10:38                     ` Junio C Hamano
2017-10-03 13:37                       ` Jeff King
2017-10-03 13:39                         ` [PATCH 01/12] test-terminal: set TERM=vt100 Jeff King
2017-10-03 13:40                         ` [PATCH 02/12] t4015: prefer --color to -c color.diff=always Jeff King
2017-10-03 13:41                         ` [PATCH 03/12] t4015: use --color with --color-moved Jeff King
2017-10-03 13:42                         ` [PATCH 04/12] t3701: use test-terminal to collect color output Jeff King
2017-10-03 13:43                         ` [PATCH 05/12] t7508: use test_terminal for " Jeff King
2017-10-03 13:43                         ` [PATCH 06/12] t7502: use diff.noprefix for --verbose test Jeff King
2017-10-03 13:44                         ` [PATCH 07/12] t6006: drop "always" color config tests Jeff King
2017-10-03 13:44                         ` [PATCH 08/12] t3203: drop "always" color test Jeff King
2017-10-03 13:44                         ` [PATCH 09/12] t7301: use test_terminal to check color Jeff King
2017-10-03 13:45                         ` [PATCH 10/12] t3205: use --color instead of color.branch=always Jeff King
2017-10-03 13:45                         ` [PATCH 11/12] provide --color option for all ref-filter users Jeff King
2017-10-03 13:46                         ` Jeff King [this message]
2017-10-04  2:59                         ` Updated to v2.14.2 on macOS; git add --patch broken Junio C Hamano
2017-10-05 10:06                           ` Jeff King
2017-10-20 13:31                     ` Jan Prachař
2017-10-03  9:31         ` Jeff King
2017-10-03  4:07   ` [PATCH 0/2] fixing "add -p" regression Junio C Hamano
2017-10-03  4:07     ` [PATCH 1/2] Revert "color: check color.ui in git_default_config()" Junio C Hamano
2017-10-03  4:07     ` [PATCH 2/2] colors: git_default_config() does not read color.ui Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171003134606.2r2653znqidt2mgx@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=ttmost@gmail.com \
    --cc=tuebernickel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).