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

Re: [PATCH 4/7] parse_color: refactor color storage

From
Jeff King <peff@peff.net>
Date
Dec 9, 2014, 21:01 UTC
Message-ID
<20141209210126.GA14305@peff.net>
In-Reply-To
<548760FA.50209@kdbg.org>
On Tue, Dec 09, 2014 at 09:52:10PM +0100, Johannes Sixt wrote:
> This patch would actually be my personal preference. The comment near
> the literal makes it all clear.
Thanks. Here it is ready to be applied.
Junio, this goes on top of the jk/colors topic.
-- >8 --
Subject: [PATCH] parse_color: drop COLOR_BACKGROUND macro

Commit 695d95d (parse_color: refactor color storage, 2014-11-20) introduced two macros, COLOR_FOREGROUND and COLOR_BACKGROUND. The latter conflicts with a system macro defined on Windows, breaking compilation there.

The simplest solution is to just get rid of these macros entirely. They are constants that are only used in one place (since the whole point of 695d95d was to avoid repeating ourselves). Their main function is to make the magic character constants more readable, but we can do the same thing with a comment.

Reported-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Jeff King <peff@peff.net>
---
 color.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/color.c b/color.c
index e2a0a99..809b359 100644
--- a/color.c
+++ b/color.c
@@ -144,9 +144,6 @@ int color_parse(const char *value, char *dst)
 	return color_parse_mem(value, strlen(value), dst);
 }
 
-#define COLOR_FOREGROUND '3'
-#define COLOR_BACKGROUND '4'
-
 /*
  * Write the ANSI color codes for "c" to "out"; the string should
  * already have the ANSI escape code in it. "out" should have enough
@@ -245,12 +242,14 @@ int color_parse_mem(const char *value, int value_len, char *dst)
 		if (!color_empty(&fg)) {
 			if (sep++)
 				*dst++ = ';';
-			dst = color_output(dst, &fg, COLOR_FOREGROUND);
+			/* foreground colors are all in the 3x range */
+			dst = color_output(dst, &fg, '3');
 		}
 		if (!color_empty(&bg)) {
 			if (sep++)
 				*dst++ = ';';
-			dst = color_output(dst, &bg, COLOR_BACKGROUND);
+			/* background colors are all in the 4x range */
+			dst = color_output(dst, &bg, '4');
 		}
 		*dst++ = 'm';
 	}
-- 
2.2.0.454.g7eca6b7
Previous: Johannes SixtNext: Eric Sunshine
Message 14 of 22 in “Re: diff-highlight highlight words?”
  1. Jeff KingNov 12, 2014
  2. Scott BakerNov 12, 2014
  3. 0/7 color fixes and configurable diff-highlightJeff King, Nov 20, 2014
  4. 1/7 docs: describe ANSI 256-color modeJeff King, Nov 20, 2014
  5. 2/7 config: fix parsing of "git config --get-color some.key -1"Jeff King, Nov 20, 2014
  6. 3/7 t4026: test "normal" colorJeff King, Nov 20, 2014
  7. Junio C HamanoNov 20, 2014
  8. Jeff KingNov 20, 2014
  9. 4/7 parse_color: refactor color storageJeff King, Nov 20, 2014
  10. Junio C HamanoNov 20, 2014
  11. Johannes SixtDec 9, 2014
  12. Jeff KingDec 9, 2014
  13. Johannes SixtDec 9, 2014
  14. Jeff KingDec 9, 2014
  15. Eric SunshineDec 9, 2014
  16. 5/7 parse_color: support 24-bit RGB valuesJeff King, Nov 20, 2014
  17. Junio C HamanoNov 20, 2014
  18. Jeff KingNov 20, 2014
  19. Junio C HamanoNov 20, 2014
  20. 6/7 parse_color: recognize "no$foo" to clear the $foo attributeJeff King, Nov 20, 2014
  21. Junio C HamanoNov 20, 2014
  22. 7/7 diff-highlight: allow configurable colorsJeff King, Nov 20, 2014

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.