git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>,
	"Stefan Beller" <sbeller@google.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [RFC PATCH 4/4] color.ui config: add "isatty" setting
Date: Wed, 30 May 2018 21:06:41 +0000	[thread overview]
Message-ID: <20180530210641.19771-5-avarab@gmail.com> (raw)
In-Reply-To: <20180530210641.19771-1-avarab@gmail.com>

A co-worker of mine who was using UNIX systems when dinosaurs roamed
the earth was lamenting that kids these days were using tools like
"git" that thought they knew better than isatty(3) when deciding
whether or not something was a terminal, and the state of the
documentation fixed earlier in this series certainly didn't help.

So this setting is a small gift to all the UNIX graybeards out
there. Now they can set color.ui=isatty and only emit fancy colors in
situations when the gods of old intended, not whatever heuristic we've
decided to set "auto" to.

As noted here this is *currently* the same as setting color.ui=auto &
color.pager=false, but I think it's good to explicitly have this
setting for any future changes. The reason, as now noted in the
documentation is that the "auto" setting may become even smarter in
the future and learn even deeper heuristics for when to turn itself on
even if isatty(3) were returning true.

At that point the fans of plain isatty(3) will become even more upset
at what we're doing, so let's give them a simple future-proof opt-out.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/config.txt | 10 ++++++++++
 color.c                  | 12 ++++++++----
 color.h                  |  1 +
 t/t7006-pager.sh         | 15 +++++++++++++++
 4 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index b882a88214..183569786f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1292,6 +1292,16 @@ color.ui::
 	determined by a call to `isatty(3)`) or to a pager (unless
 	`color.pager` is set to false).
 +
+If you don't like the magic of `auto` and prefer for git to just ask
+`isatty(3)` whether it's connected to a terminal set this to
+`isatty`. This will cause it to always obey that function, except
+(like `auto`) if `TERM=dumb` is set in the environment. Currently this
+is equivalent to setting `auto` and `color.pager=false`, but in the
+future `auto` may be smart enough to handle other cases, i.e. when
+`isatty(3)` returns `1` but something else other than `TERM=dumb`
+suggests the terminal can't handle colors or not. If you'd like to
+avoid all that magic use `isatty`.
++
 Setting this to some value unknown to git will warn and fall back to
 `auto`. This is so that new values can be recognized in the future
 without the git configuration file being incompatible between versions
diff --git a/color.c b/color.c
index e52c6cdd29..05f95649bc 100644
--- a/color.c
+++ b/color.c
@@ -306,6 +306,8 @@ int git_config_colorbool(const char *var, const char *value)
 			return 1;
 		if (!strcasecmp(value, "auto"))
 			return GIT_COLOR_AUTO;
+		if (!strcasecmp(value, "isatty"))
+			return GIT_COLOR_ISATTY;
 	}
 
 	if (!var)
@@ -332,13 +334,14 @@ int git_config_colorbool(const char *var, const char *value)
 	return GIT_COLOR_AUTO;
 }
 
-static int check_auto_color(int fd)
+static int check_auto_color(int fd, int isatty_only)
 {
 	static int color_stderr_is_tty = -1;
 	int *is_tty_p = fd == 1 ? &color_stdout_is_tty : &color_stderr_is_tty;
 	if (*is_tty_p < 0)
 		*is_tty_p = isatty(fd);
-	if (*is_tty_p || (fd == 1 && pager_in_use() && pager_use_color)) {
+	if (*is_tty_p || (!isatty_only && fd == 1 && pager_in_use() &&
+			  pager_use_color)) {
 		if (!is_terminal_dumb())
 			return 1;
 	}
@@ -359,9 +362,10 @@ int want_color_fd(int fd, int var)
 	if (var < 0)
 		var = git_use_color_default;
 
-	if (var == GIT_COLOR_AUTO) {
+	if (var == GIT_COLOR_AUTO || var == GIT_COLOR_ISATTY) {
+		int isatty_only = var == GIT_COLOR_ISATTY;
 		if (want_auto[fd] < 0)
-			want_auto[fd] = check_auto_color(fd);
+			want_auto[fd] = check_auto_color(fd, isatty_only);
 		return want_auto[fd];
 	}
 	return var;
diff --git a/color.h b/color.h
index 5b744e1bc6..01d8cc01a5 100644
--- a/color.h
+++ b/color.h
@@ -57,6 +57,7 @@ struct strbuf;
 #define GIT_COLOR_NEVER  0
 #define GIT_COLOR_ALWAYS 1
 #define GIT_COLOR_AUTO   2
+#define GIT_COLOR_ISATTY 3
 
 /* A default list of colors to use for commit graphs and show-branch output */
 extern const char *column_colors_ansi[];
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index b16f2ac28b..ea4f2f47d0 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -309,6 +309,13 @@ test_expect_success 'no color when stdout is a regular file' '
 	! colorful colorless.log
 '
 
+test_expect_success 'no color when stdout is a regular file (isatty)' '
+	rm -f colorless.log &&
+	test_config color.ui isatty &&
+	git log >colorless.log &&
+	! colorful colorless.log
+'
+
 test_expect_success 'unknown color.ui values default to "auto" (regular file)' '
 	rm -f colorless.log &&
 	test_config color.ui doesnotexist &&
@@ -340,6 +347,14 @@ test_expect_success TTY 'colors are suppressed by color.pager' '
 	! colorful paginated.out
 '
 
+test_expect_success TTY 'colors are suppressed by color.ui=isatty when writing to a pager' '
+	rm -f paginated.out &&
+	test_config color.ui isatty &&
+	test_config color.pager true &&
+	test_terminal git log &&
+	! colorful paginated.out
+'
+
 test_expect_success 'color when writing to a file intended for a pager' '
 	rm -f colorful.log &&
 	test_config color.ui auto &&
-- 
2.17.0.290.gded63e768a


  parent reply	other threads:[~2018-05-30 21:07 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-30 21:06 [PATCH 0/4] color.ui docs & add color.ui=isatty Ævar Arnfjörð Bjarmason
2018-05-30 21:06 ` [PATCH 1/4] config doc: move color.ui documentation to one place Ævar Arnfjörð Bjarmason
2018-05-31  5:25   ` Jeff King
2018-05-31  7:09     ` Ævar Arnfjörð Bjarmason
2018-06-01  5:31       ` Jeff King
2018-05-30 21:06 ` [PATCH 2/4] config doc: clarify "to a terminal" in color.ui Ævar Arnfjörð Bjarmason
2018-05-31  5:27   ` Jeff King
2018-05-30 21:06 ` [RFC PATCH 3/4] color.ui config: don't die on unknown values Ævar Arnfjörð Bjarmason
2018-05-30 22:32   ` Stefan Beller
2018-05-30 23:05   ` Junio C Hamano
2018-05-31  7:17     ` Ævar Arnfjörð Bjarmason
2018-06-01  5:53       ` Jeff King
2018-05-30 21:06 ` Ævar Arnfjörð Bjarmason [this message]
2018-05-30 22:57   ` [RFC PATCH 4/4] color.ui config: add "isatty" setting Junio C Hamano
2018-05-31  7:07     ` Ævar Arnfjörð Bjarmason
2018-05-31  5:38   ` Jeff King
2018-05-31  7:01     ` Ævar Arnfjörð Bjarmason
2018-06-01  5:30       ` Jeff King

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=20180530210641.19771-5-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=peff@peff.net \
    --cc=sbeller@google.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).