git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Steffen Daode Nurpmeso" <sdaoden@googlemail.com>,
	"Ingo Brückl" <ib@wupperonline.de>
Subject: [PATCH 06/10] git_config_colorbool: refactor stdout_is_tty handling
Date: Wed, 17 Aug 2011 22:03:48 -0700	[thread overview]
Message-ID: <20110818050346.GF2889@sigill.intra.peff.net> (raw)
In-Reply-To: <20110818045821.GA17377@sigill.intra.peff.net>

Usually this function figures out for itself whether stdout
is a tty. However, it has an extra parameter just to allow
git-config to override the auto-detection for its
--get-colorbool option.

Instead of an extra parameter, let's just use a global
variable. This makes calling easier in the common case, and
will make refactoring the colorbool code much simpler.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/branch.c      |    2 +-
 builtin/commit.c      |    2 +-
 builtin/config.c      |   23 +++++++----------------
 builtin/grep.c        |    2 +-
 builtin/show-branch.c |    2 +-
 color.c               |   11 ++++++-----
 color.h               |    8 +++++++-
 diff.c                |    4 ++--
 parse-options.c       |    2 +-
 9 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 3142daa..b15fee5 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -71,7 +71,7 @@ static int parse_branch_color_slot(const char *var, int ofs)
 static int git_branch_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "color.branch")) {
-		branch_use_color = git_config_colorbool(var, value, -1);
+		branch_use_color = git_config_colorbool(var, value);
 		return 0;
 	}
 	if (!prefixcmp(var, "color.branch.")) {
diff --git a/builtin/commit.c b/builtin/commit.c
index e1af9b1..295803a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1144,7 +1144,7 @@ static int git_status_config(const char *k, const char *v, void *cb)
 		return 0;
 	}
 	if (!strcmp(k, "status.color") || !strcmp(k, "color.status")) {
-		s->use_color = git_config_colorbool(k, v, -1);
+		s->use_color = git_config_colorbool(k, v);
 		return 0;
 	}
 	if (!prefixcmp(k, "status.color.") || !prefixcmp(k, "color.status.")) {
diff --git a/builtin/config.c b/builtin/config.c
index 211e118..5505ced 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -303,24 +303,17 @@ static void get_color(const char *def_color)
 	fputs(parsed_color, stdout);
 }
 
-static int stdout_is_tty;
 static int get_colorbool_found;
 static int get_diff_color_found;
 static int git_get_colorbool_config(const char *var, const char *value,
 		void *cb)
 {
-	if (!strcmp(var, get_colorbool_slot)) {
-		get_colorbool_found =
-			git_config_colorbool(var, value, stdout_is_tty);
-	}
-	if (!strcmp(var, "diff.color")) {
-		get_diff_color_found =
-			git_config_colorbool(var, value, stdout_is_tty);
-	}
-	if (!strcmp(var, "color.ui")) {
-		git_use_color_default = git_config_colorbool(var, value, stdout_is_tty);
-		return 0;
-	}
+	if (!strcmp(var, get_colorbool_slot))
+		get_colorbool_found = git_config_colorbool(var, value);
+	else if (!strcmp(var, "diff.color"))
+		get_diff_color_found = git_config_colorbool(var, value);
+	else if (!strcmp(var, "color.ui"))
+		git_use_color_default = git_config_colorbool(var, value);
 	return 0;
 }
 
@@ -510,9 +503,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	}
 	else if (actions == ACTION_GET_COLORBOOL) {
 		if (argc == 1)
-			stdout_is_tty = git_config_bool("command line", argv[0]);
-		else if (argc == 0)
-			stdout_is_tty = isatty(1);
+			color_stdout_is_tty = git_config_bool("command line", argv[0]);
 		return get_colorbool(argc != 0);
 	}
 
diff --git a/builtin/grep.c b/builtin/grep.c
index cccf8da..d80db22 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -325,7 +325,7 @@ static int grep_config(const char *var, const char *value, void *cb)
 	}
 
 	if (!strcmp(var, "color.grep"))
-		opt->color = git_config_colorbool(var, value, -1);
+		opt->color = git_config_colorbool(var, value);
 	else if (!strcmp(var, "color.grep.context"))
 		color = opt->color_context;
 	else if (!strcmp(var, "color.grep.filename"))
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index facc63a..e6650b4 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -573,7 +573,7 @@ static int git_show_branch_config(const char *var, const char *value, void *cb)
 	}
 
 	if (!strcmp(var, "color.showbranch")) {
-		showbranch_use_color = git_config_colorbool(var, value, -1);
+		showbranch_use_color = git_config_colorbool(var, value);
 		return 0;
 	}
 
diff --git a/color.c b/color.c
index 3db214c..67affa4 100644
--- a/color.c
+++ b/color.c
@@ -2,6 +2,7 @@
 #include "color.h"
 
 int git_use_color_default = 0;
+int color_stdout_is_tty = -1;
 
 /*
  * The list of available column colors.
@@ -157,7 +158,7 @@ bad:
 	die("bad color value '%.*s' for variable '%s'", value_len, value, var);
 }
 
-int git_config_colorbool(const char *var, const char *value, int stdout_is_tty)
+int git_config_colorbool(const char *var, const char *value)
 {
 	if (value) {
 		if (!strcasecmp(value, "never"))
@@ -177,9 +178,9 @@ int git_config_colorbool(const char *var, const char *value, int stdout_is_tty)
 
 	/* any normal truth value defaults to 'auto' */
  auto_color:
-	if (stdout_is_tty < 0)
-		stdout_is_tty = isatty(1);
-	if (stdout_is_tty || (pager_in_use() && pager_use_color)) {
+	if (color_stdout_is_tty < 0)
+		color_stdout_is_tty = isatty(1);
+	if (color_stdout_is_tty || (pager_in_use() && pager_use_color)) {
 		char *term = getenv("TERM");
 		if (term && strcmp(term, "dumb"))
 			return 1;
@@ -190,7 +191,7 @@ int git_config_colorbool(const char *var, const char *value, int stdout_is_tty)
 int git_color_default_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "color.ui")) {
-		git_use_color_default = git_config_colorbool(var, value, -1);
+		git_use_color_default = git_config_colorbool(var, value);
 		return 0;
 	}
 
diff --git a/color.h b/color.h
index 68a926a..a190a25 100644
--- a/color.h
+++ b/color.h
@@ -58,11 +58,17 @@ extern const char *column_colors_ansi[];
 extern const int column_colors_ansi_max;
 
 /*
+ * Generally the color code will lazily figure this out itself, but
+ * this provides a mechanism for callers to override autodetection.
+ */
+extern int color_stdout_is_tty;
+
+/*
  * Use this instead of git_default_config if you need the value of color.ui.
  */
 int git_color_default_config(const char *var, const char *value, void *cb);
 
-int git_config_colorbool(const char *var, const char *value, int stdout_is_tty);
+int git_config_colorbool(const char *var, const char *value);
 void color_parse(const char *value, const char *var, char *dst);
 void color_parse_mem(const char *value, int len, const char *var, char *dst);
 __attribute__((format (printf, 3, 4)))
diff --git a/diff.c b/diff.c
index 2d86abe..2dfc359 100644
--- a/diff.c
+++ b/diff.c
@@ -137,7 +137,7 @@ static int git_config_rename(const char *var, const char *value)
 int git_diff_ui_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
-		diff_use_color_default = git_config_colorbool(var, value, -1);
+		diff_use_color_default = git_config_colorbool(var, value);
 		return 0;
 	}
 	if (!strcmp(var, "diff.renames")) {
@@ -3409,7 +3409,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 	else if (!strcmp(arg, "--color"))
 		options->use_color = 1;
 	else if (!prefixcmp(arg, "--color=")) {
-		int value = git_config_colorbool(NULL, arg+8, -1);
+		int value = git_config_colorbool(NULL, arg+8);
 		if (value == 0)
 			options->use_color = 0;
 		else if (value > 0)
diff --git a/parse-options.c b/parse-options.c
index 879ea82..be4383e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -621,7 +621,7 @@ int parse_opt_color_flag_cb(const struct option *opt, const char *arg,
 
 	if (!arg)
 		arg = unset ? "never" : (const char *)opt->defval;
-	value = git_config_colorbool(NULL, arg, -1);
+	value = git_config_colorbool(NULL, arg);
 	if (value < 0)
 		return opterror(opt,
 			"expects \"always\", \"auto\", or \"never\"", 0);
-- 
1.7.6.10.g62f04

  parent reply	other threads:[~2011-08-18  5:03 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-18  4:58 [PATCH 0/10] color and pager improvements Jeff King
2011-08-18  5:00 ` [PATCH 01/10] t7006: modernize calls to unset Jeff King
2011-08-18 21:05   ` Junio C Hamano
2011-08-18  5:01 ` [PATCH 02/10] test-lib: add helper functions for config Jeff King
2011-08-18 21:10   ` Junio C Hamano
2011-08-18  5:02 ` [PATCH 03/10] t7006: use test_config helpers Jeff King
2011-08-18  5:02 ` [PATCH 04/10] setup_pager: set GIT_PAGER_IN_USE Jeff King
2011-08-18  5:03 ` [PATCH 05/10] diff: refactor COLOR_DIFF from a flag into an int Jeff King
2011-08-18  5:03 ` Jeff King [this message]
2011-08-18  5:04 ` [PATCH 07/10] color: delay auto-color decision until point of use Jeff King
2011-08-18 21:59   ` Junio C Hamano
2011-08-18 22:28     ` Jeff King
2011-08-18  5:04 ` [PATCH 08/10] config: refactor get_colorbool function Jeff King
2011-08-18  5:05 ` [PATCH 09/10] diff: don't load color config in plumbing Jeff King
2011-08-18  5:05 ` [PATCH 10/10] want_color: automatically fallback to color.ui Jeff King
2011-09-04  2:36   ` Martin von Zweigbergk
2011-09-04 12:53     ` Jeff King
2011-09-05 11:31       ` Steffen Daode Nurpmeso
2011-08-18 21:58 ` [PATCH 0/10] color and pager improvements Jeff King
2011-08-18 21:59   ` [PATCH 11/10] support pager.* for aliases Jeff King
2011-08-18 22:54     ` Junio C Hamano
2011-08-19  3:37       ` Jeff King
2011-08-19  4:18         ` Junio C Hamano
2011-08-19  4:40           ` Jeff King
2011-08-19  5:23             ` Junio C Hamano
2011-08-19  5:43               ` Junio C Hamano
2011-08-19  8:30               ` Jeff King
2011-08-18 22:01   ` [PATCH 12/10] support pager.* for external commands Jeff King
2011-08-18 22:56     ` Junio C Hamano
2012-02-12  0:46     ` Ævar Arnfjörð Bjarmason
2012-02-14 19:13       ` Jeff King
2011-08-18 22:33   ` [PATCH 0/10] color and pager improvements Ingo Brückl
2011-08-18 22:46     ` Jeff King
2011-08-19  6:34       ` Ingo Brückl
2011-08-25 20:25         ` Jeff King
2011-08-18 21:59 ` Steffen Daode Nurpmeso
2011-08-18 22:02 ` 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

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

  git send-email \
    --in-reply-to=20110818050346.GF2889@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=ib@wupperonline.de \
    --cc=sdaoden@googlemail.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.
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 NNTP newsgroup(s).