git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 10/10] want_color: automatically fallback to color.ui

From
Jeff King <peff@peff.net>
Date
Aug 18, 2011, 05:05 UTC
Message-ID
<20110818050533.GJ2889@sigill.intra.peff.net>
In-Reply-To
<20110818045821.GA17377@sigill.intra.peff.net>

All of the "do we want color" flags default to -1 to indicate that we don't have any color configured. This value is handled in one of two ways:

  1. In porcelain, we check early on whether the value is
     still -1 after reading the config, and set it to the
     value of color.ui (which defaults to 0).
  2. In plumbing, it stays untouched as -1, and want_color
     defaults it to off.

This works fine, but means that every porcelain has to check and reassign its color flag. Now that want_color gives us a place to put this check in a single spot, we can do that, simplifying the calling code.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/branch.c      |    3 ---
 builtin/commit.c      |   11 +----------
 builtin/diff.c        |    3 ---
 builtin/grep.c        |    2 --
 builtin/log.c         |   12 ------------
 builtin/merge.c       |    4 ----
 builtin/show-branch.c |    3 ---
 color.c               |    7 +++++--
 color.h               |    5 -----
 9 files changed, 6 insertions(+), 44 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index d6d3c7d..73d4170 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -673,9 +673,6 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 	git_config(git_branch_config, NULL);
 
-	if (branch_use_color == -1)
-		branch_use_color = git_use_color_default;
-
 	track = git_branch_track;
 
 	head = resolve_ref("HEAD", head_sha1, 0, NULL);
diff --git a/builtin/commit.c b/builtin/commit.c
index 295803a..9763146 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1237,10 +1237,6 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	if (s.relative_paths)
 		s.prefix = prefix;
-	if (s.use_color == -1)
-		s.use_color = git_use_color_default;
-	if (diff_use_color_default == -1)
-		diff_use_color_default = git_use_color_default;
 
 	switch (status_format) {
 	case STATUS_FORMAT_SHORT:
@@ -1394,15 +1390,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	git_config(git_commit_config, &s);
 	determine_whence(&s);
 
-	if (s.use_color == -1)
-		s.use_color = git_use_color_default;
 	argc = parse_and_validate_options(argc, argv, builtin_commit_usage,
 					  prefix, &s);
-	if (dry_run) {
-		if (diff_use_color_default == -1)
-			diff_use_color_default = git_use_color_default;
+	if (dry_run)
 		return dry_run_commit(argc, argv, prefix, &s);
-	}
 	index_file = prepare_index(argc, argv, prefix, 0);
 
 	/* Set up everything for writing the commit object.  This includes
diff --git a/builtin/diff.c b/builtin/diff.c
index 69cd5ee..1118689 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -277,9 +277,6 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	gitmodules_config();
 	git_config(git_diff_ui_config, NULL);
 
-	if (diff_use_color_default == -1)
-		diff_use_color_default = git_use_color_default;
-
 	init_revisions(&rev, prefix);
 
 	/* If this is a no-index diff, just run it and exit there. */
diff --git a/builtin/grep.c b/builtin/grep.c
index d80db22..2cbf01f 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -896,8 +896,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	strcpy(opt.color_sep, GIT_COLOR_CYAN);
 	opt.color = -1;
 	git_config(grep_config, &opt);
-	if (opt.color == -1)
-		opt.color = git_use_color_default;
 
 	/*
 	 * If there is no -- then the paths must exist in the working
diff --git a/builtin/log.c b/builtin/log.c
index 5c2af59..d760ee0 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -359,9 +359,6 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
 
 	git_config(git_log_config, NULL);
 
-	if (diff_use_color_default == -1)
-		diff_use_color_default = git_use_color_default;
-
 	init_revisions(&rev, prefix);
 	rev.diff = 1;
 	rev.simplify_history = 0;
@@ -446,9 +443,6 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 
 	git_config(git_log_config, NULL);
 
-	if (diff_use_color_default == -1)
-		diff_use_color_default = git_use_color_default;
-
 	init_pathspec(&match_all, NULL);
 	init_revisions(&rev, prefix);
 	rev.diff = 1;
@@ -524,9 +518,6 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
 
 	git_config(git_log_config, NULL);
 
-	if (diff_use_color_default == -1)
-		diff_use_color_default = git_use_color_default;
-
 	init_revisions(&rev, prefix);
 	init_reflog_walk(&rev.reflog_info);
 	rev.verbose_header = 1;
@@ -549,9 +540,6 @@ int cmd_log(int argc, const char **argv, const char *prefix)
 
 	git_config(git_log_config, NULL);
 
-	if (diff_use_color_default == -1)
-		diff_use_color_default = git_use_color_default;
-
 	init_revisions(&rev, prefix);
 	rev.always_show_header = 1;
 	memset(&opt, 0, sizeof(opt));
diff --git a/builtin/merge.c b/builtin/merge.c
index 7209edf..b75ae01 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1031,10 +1031,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 	git_config(git_merge_config, NULL);
 
-	/* for color.ui */
-	if (diff_use_color_default == -1)
-		diff_use_color_default = git_use_color_default;
-
 	if (branch_mergeoptions)
 		parse_branch_merge_options(branch_mergeoptions);
 	argc = parse_options(argc, argv, prefix, builtin_merge_options,
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 4b726fa..4b480d7 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -685,9 +685,6 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 
 	git_config(git_show_branch_config, NULL);
 
-	if (showbranch_use_color == -1)
-		showbranch_use_color = git_use_color_default;
-
 	/* If nothing is specified, try the default first */
 	if (ac == 1 && default_num) {
 		ac = default_num;
diff --git a/color.c b/color.c
index ec96fe1..e8e2681 100644
--- a/color.c
+++ b/color.c
@@ -1,7 +1,7 @@
 #include "cache.h"
 #include "color.h"
 
-int git_use_color_default = 0;
+static int git_use_color_default = 0;
 int color_stdout_is_tty = -1;
 
 /*
@@ -196,12 +196,15 @@ int want_color(int var)
 {
 	static int want_auto = -1;
 
+	if (var < 0)
+		var = git_use_color_default;
+
 	if (var == GIT_COLOR_AUTO) {
 		if (want_auto < 0)
 			want_auto = check_auto_color();
 		return want_auto;
 	}
-	return var > 0;
+	return var;
 }
 
 int git_color_config(const char *var, const char *value, void *cb)
diff --git a/color.h b/color.h
index 5949bcd..3068a99 100644
--- a/color.h
+++ b/color.h
@@ -58,11 +58,6 @@ struct strbuf;
 #define GIT_COLOR_NEVER  1
 #define GIT_COLOR_AUTO   2
 
-/*
- * This variable stores the value of color.ui
- */
-extern int git_use_color_default;
-
 /* A default list of colors to use for commit graphs and show-branch output */
 extern const char *column_colors_ansi[];
 extern const int column_colors_ansi_max;

-- 
1.7.6.10.g62f04
Previous: Jeff KingNext: Martin von Zweigbergk
Message 15 of 37 in “color and pager improvements”
  1. 0/10 color and pager improvementsJeff King, Aug 18, 2011
  2. 01/10 t7006: modernize calls to unsetJeff King, Aug 18, 2011
  3. Junio C HamanoAug 18, 2011
  4. 02/10 test-lib: add helper functions for configJeff King, Aug 18, 2011
  5. Junio C HamanoAug 18, 2011
  6. 03/10 t7006: use test_config helpersJeff King, Aug 18, 2011
  7. 04/10 setup_pager: set GIT_PAGER_IN_USEJeff King, Aug 18, 2011
  8. 05/10 diff: refactor COLOR_DIFF from a flag into an intJeff King, Aug 18, 2011
  9. 06/10 git_config_colorbool: refactor stdout_is_tty handlingJeff King, Aug 18, 2011
  10. 07/10 color: delay auto-color decision until point of useJeff King, Aug 18, 2011
  11. Junio C HamanoAug 18, 2011
  12. Jeff KingAug 18, 2011
  13. 08/10 config: refactor get_colorbool functionJeff King, Aug 18, 2011
  14. 09/10 diff: don't load color config in plumbingJeff King, Aug 18, 2011
  15. 10/10 want_color: automatically fallback to color.uiJeff King, Aug 18, 2011
  16. Martin von ZweigbergkSep 4, 2011
  17. Jeff KingSep 4, 2011
  18. Steffen Daode NurpmesoSep 5, 2011
  19. Jeff KingAug 18, 2011
  20. 11/10 support pager.* for aliasesJeff King, Aug 18, 2011
  21. Junio C HamanoAug 18, 2011
  22. Jeff KingAug 19, 2011
  23. Junio C HamanoAug 19, 2011
  24. Jeff KingAug 19, 2011
  25. Junio C HamanoAug 19, 2011
  26. Junio C HamanoAug 19, 2011
  27. Jeff KingAug 19, 2011
  28. 12/10 support pager.* for external commandsJeff King, Aug 18, 2011
  29. Junio C HamanoAug 18, 2011
  30. Ævar Arnfjörð BjarmasonFeb 12, 2012
  31. Jeff KingFeb 14, 2012
  32. Ingo BrücklAug 18, 2011
  33. Jeff KingAug 18, 2011
  34. Ingo BrücklAug 19, 2011
  35. Jeff KingAug 25, 2011
  36. Steffen Daode NurpmesoAug 18, 2011
  37. Junio C HamanoAug 18, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.