{"thread":{"id":"5504","subject":"[PATCH 2/3] Move color option parsing out of diff.c and into color.[ch]","startedAt":"2006-09-07T06:35:59Z","lastAt":"2006-09-08T00:11:32Z","messageCount":3,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"26485","messageId":"20060907063559.GB17083@coredump.intra.peff.net","threadId":"5504","inReplyTo":"2ec783f6a8e8a901f7c30947e8c0eb50f71bc185.1157610743.git.peff@peff.net","subject":"[PATCH 2/3] Move color option parsing out of diff.c and into color.[ch]","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2006-09-07T06:35:59Z","receivedAt":"2006-09-07T06:35:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The intent is to lib-ify colorizing code so it can be reused.\n---\n Makefile |    3 +\n color.c  |  158 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n color.h  |   11 ++++\n diff.c   |  136 ++---------------------------------------------------\n 4 files changed, 175 insertions(+), 133 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 7b3114f..78748cb 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -251,7 +251,8 @@ LIB_OBJS = \\\n \ttag.o tree.o usage.o config.o environment.o ctype.o copy.o \\\n \tfetch-clone.o revision.o pager.o tree-walk.o xdiff-interface.o \\\n \twrite_or_die.o trace.o \\\n-\talloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS)\n+\talloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \\\n+\tcolor.o\n \n BUILTIN_OBJS = \\\n \tbuiltin-add.o \\\ndiff --git a/color.c b/color.c\nnew file mode 100644\nindex 0000000..563ef70\n--- /dev/null\n+++ b/color.c\n@@ -0,0 +1,158 @@\n+#include \"color.h\"\n+#include \"cache.h\"\n+#include \"git-compat-util.h\"\n+\n+#include <stdarg.h>\n+\n+#define COLOR_RESET \"\\033[m\"\n+\n+static int\n+parse_color(const char *name, int len)\n+{\n+\tstatic const char * const color_names[] = {\n+\t\t\"normal\", \"black\", \"red\", \"green\", \"yellow\",\n+\t\t\"blue\", \"magenta\", \"cyan\", \"white\"\n+\t};\n+\tchar *end;\n+\tint i;\n+\tfor (i = 0; i < ARRAY_SIZE(color_names); i++) {\n+\t\tconst char *str = color_names[i];\n+\t\tif (!strncasecmp(name, str, len) && !str[len])\n+\t\t\treturn i - 1;\n+\t}\n+\ti = strtol(name, &end, 10);\n+\tif (*name && !*end && i >= -1 && i <= 255)\n+\t\treturn i;\n+\treturn -2;\n+}\n+\n+static int\n+parse_attr(const char *name, int len)\n+{\n+\tstatic const int attr_values[] = { 1, 2, 4, 5, 7 };\n+\tstatic const char * const attr_names[] = {\n+\t\t\"bold\", \"dim\", \"ul\", \"blink\", \"reverse\"\n+\t};\n+\tint i;\n+\tfor (i = 0; i < ARRAY_SIZE(attr_names); i++) {\n+\t\tconst char *str = attr_names[i];\n+\t\tif (!strncasecmp(name, str, len) && !str[len])\n+\t\t\treturn attr_values[i];\n+\t}\n+\treturn -1;\n+}\n+\n+void\n+color_parse(const char *value, const char *var, char *dst)\n+{\n+\tconst char *ptr = value;\n+\tint attr = -1;\n+\tint fg = -2;\n+\tint bg = -2;\n+\n+\tif (!strcasecmp(value, \"reset\")) {\n+\t\tstrcpy(dst, \"\\033[m\");\n+\t\treturn;\n+\t}\n+\n+\t/* [fg [bg]] [attr] */\n+\twhile (*ptr) {\n+\t\tconst char *word = ptr;\n+\t\tint val, len = 0;\n+\n+\t\twhile (word[len] && !isspace(word[len]))\n+\t\t\tlen++;\n+\n+\t\tptr = word + len;\n+\t\twhile (*ptr && isspace(*ptr))\n+\t\t\tptr++;\n+\n+\t\tval = parse_color(word, len);\n+\t\tif (val >= -1) {\n+\t\t\tif (fg == -2) {\n+\t\t\t\tfg = val;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tif (bg == -2) {\n+\t\t\t\tbg = val;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tgoto bad;\n+\t\t}\n+\t\tval = parse_attr(word, len);\n+\t\tif (val < 0 || attr != -1)\n+\t\t\tgoto bad;\n+\t\tattr = val;\n+\t}\n+\n+\tif (attr >= 0 || fg >= 0 || bg >= 0) {\n+\t\tint sep = 0;\n+\n+\t\t*dst++ = '\\033';\n+\t\t*dst++ = '[';\n+\t\tif (attr >= 0) {\n+\t\t\t*dst++ = '0' + attr;\n+\t\t\tsep++;\n+\t\t}\n+\t\tif (fg >= 0) {\n+\t\t\tif (sep++)\n+\t\t\t\t*dst++ = ';';\n+\t\t\tif (fg < 8) {\n+\t\t\t\t*dst++ = '3';\n+\t\t\t\t*dst++ = '0' + fg;\n+\t\t\t} else {\n+\t\t\t\tdst += sprintf(dst, \"38;5;%d\", fg);\n+\t\t\t}\n+\t\t}\n+\t\tif (bg >= 0) {\n+\t\t\tif (sep++)\n+\t\t\t\t*dst++ = ';';\n+\t\t\tif (bg < 8) {\n+\t\t\t\t*dst++ = '4';\n+\t\t\t\t*dst++ = '0' + bg;\n+\t\t\t} else {\n+\t\t\t\tdst += sprintf(dst, \"48;5;%d\", bg);\n+\t\t\t}\n+\t\t}\n+\t\t*dst++ = 'm';\n+\t}\n+\t*dst = 0;\n+\treturn;\n+bad:\n+\tdie(\"bad config value '%s' for variable '%s'\", value, var);\n+}\n+\n+int\n+git_config_colorbool(const char *var, const char *value)\n+{\n+\tif (!value)\n+\t\treturn 1;\n+\tif (!strcasecmp(value, \"auto\")) {\n+\t\tif (isatty(1) || (pager_in_use && pager_use_color)) {\n+\t\t\tchar *term = getenv(\"TERM\");\n+\t\t\tif (term && strcmp(term, \"dumb\"))\n+\t\t\t\treturn 1;\n+\t\t}\n+\t\treturn 0;\n+\t}\n+\tif (!strcasecmp(value, \"never\"))\n+\t\treturn 0;\n+\tif (!strcasecmp(value, \"always\"))\n+\t\treturn 1;\n+\treturn git_config_bool(var, value);\n+}\n+\n+int\n+color_printf(const char *color, const char *fmt, ...) {\n+\tva_list args;\n+\tint r;\n+\n+\tif (*color)\n+\t\tprintf(\"%s\", color);\n+\tva_start(args, fmt);\n+\tr = vprintf(fmt, args);\n+\tva_end(args);\n+\tif (*color)\n+\t\tprintf(\"%s\", COLOR_RESET);\n+\treturn r;\n+}\ndiff --git a/color.h b/color.h\nnew file mode 100644\nindex 0000000..37a737a\n--- /dev/null\n+++ b/color.h\n@@ -0,0 +1,11 @@\n+#ifndef COLOR_H\n+#define COLOR_H\n+\n+/* \"\\033[1;38;5;2xx;48;5;2xxm\\0\" is 23 bytes */\n+#define COLOR_MAXLEN 24\n+\n+int git_config_colorbool(const char *var, const char *value);\n+void color_parse(const char *var, const char *value, char *dst);\n+int color_printf(const char *color, const char *fmt, ...);\n+\n+#endif /* COLOR_H */\ndiff --git a/diff.c b/diff.c\nindex a3ebc62..8178c11 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -10,6 +10,7 @@ #include \"diff.h\"\n #include \"diffcore.h\"\n #include \"delta.h\"\n #include \"xdiff-interface.h\"\n+#include \"color.h\"\n \n static int use_size_cache;\n \n@@ -17,8 +18,7 @@ static int diff_detect_rename_default;\n static int diff_rename_limit_default = -1;\n static int diff_use_color_default;\n \n-/* \"\\033[1;38;5;2xx;48;5;2xxm\\0\" is 23 bytes */\n-static char diff_colors[][24] = {\n+static char diff_colors[][COLOR_MAXLEN] = {\n \t\"\\033[m\",\t/* reset */\n \t\"\",\t\t/* normal */\n \t\"\\033[1m\",\t/* bold */\n@@ -45,119 +45,6 @@ static int parse_diff_color_slot(const c\n \tdie(\"bad config variable '%s'\", var);\n }\n \n-static int parse_color(const char *name, int len)\n-{\n-\tstatic const char * const color_names[] = {\n-\t\t\"normal\", \"black\", \"red\", \"green\", \"yellow\",\n-\t\t\"blue\", \"magenta\", \"cyan\", \"white\"\n-\t};\n-\tchar *end;\n-\tint i;\n-\tfor (i = 0; i < ARRAY_SIZE(color_names); i++) {\n-\t\tconst char *str = color_names[i];\n-\t\tif (!strncasecmp(name, str, len) && !str[len])\n-\t\t\treturn i - 1;\n-\t}\n-\ti = strtol(name, &end, 10);\n-\tif (*name && !*end && i >= -1 && i <= 255)\n-\t\treturn i;\n-\treturn -2;\n-}\n-\n-static int parse_attr(const char *name, int len)\n-{\n-\tstatic const int attr_values[] = { 1, 2, 4, 5, 7 };\n-\tstatic const char * const attr_names[] = {\n-\t\t\"bold\", \"dim\", \"ul\", \"blink\", \"reverse\"\n-\t};\n-\tint i;\n-\tfor (i = 0; i < ARRAY_SIZE(attr_names); i++) {\n-\t\tconst char *str = attr_names[i];\n-\t\tif (!strncasecmp(name, str, len) && !str[len])\n-\t\t\treturn attr_values[i];\n-\t}\n-\treturn -1;\n-}\n-\n-static void parse_diff_color_value(const char *value, const char *var, char *dst)\n-{\n-\tconst char *ptr = value;\n-\tint attr = -1;\n-\tint fg = -2;\n-\tint bg = -2;\n-\n-\tif (!strcasecmp(value, \"reset\")) {\n-\t\tstrcpy(dst, \"\\033[m\");\n-\t\treturn;\n-\t}\n-\n-\t/* [fg [bg]] [attr] */\n-\twhile (*ptr) {\n-\t\tconst char *word = ptr;\n-\t\tint val, len = 0;\n-\n-\t\twhile (word[len] && !isspace(word[len]))\n-\t\t\tlen++;\n-\n-\t\tptr = word + len;\n-\t\twhile (*ptr && isspace(*ptr))\n-\t\t\tptr++;\n-\n-\t\tval = parse_color(word, len);\n-\t\tif (val >= -1) {\n-\t\t\tif (fg == -2) {\n-\t\t\t\tfg = val;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (bg == -2) {\n-\t\t\t\tbg = val;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tgoto bad;\n-\t\t}\n-\t\tval = parse_attr(word, len);\n-\t\tif (val < 0 || attr != -1)\n-\t\t\tgoto bad;\n-\t\tattr = val;\n-\t}\n-\n-\tif (attr >= 0 || fg >= 0 || bg >= 0) {\n-\t\tint sep = 0;\n-\n-\t\t*dst++ = '\\033';\n-\t\t*dst++ = '[';\n-\t\tif (attr >= 0) {\n-\t\t\t*dst++ = '0' + attr;\n-\t\t\tsep++;\n-\t\t}\n-\t\tif (fg >= 0) {\n-\t\t\tif (sep++)\n-\t\t\t\t*dst++ = ';';\n-\t\t\tif (fg < 8) {\n-\t\t\t\t*dst++ = '3';\n-\t\t\t\t*dst++ = '0' + fg;\n-\t\t\t} else {\n-\t\t\t\tdst += sprintf(dst, \"38;5;%d\", fg);\n-\t\t\t}\n-\t\t}\n-\t\tif (bg >= 0) {\n-\t\t\tif (sep++)\n-\t\t\t\t*dst++ = ';';\n-\t\t\tif (bg < 8) {\n-\t\t\t\t*dst++ = '4';\n-\t\t\t\t*dst++ = '0' + bg;\n-\t\t\t} else {\n-\t\t\t\tdst += sprintf(dst, \"48;5;%d\", bg);\n-\t\t\t}\n-\t\t}\n-\t\t*dst++ = 'm';\n-\t}\n-\t*dst = 0;\n-\treturn;\n-bad:\n-\tdie(\"bad config value '%s' for variable '%s'\", value, var);\n-}\n-\n /*\n  * These are to give UI layer defaults.\n  * The core-level commands such as git-diff-files should\n@@ -171,22 +58,7 @@ int git_diff_ui_config(const char *var, \n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"diff.color\")) {\n-\t\tif (!value)\n-\t\t\tdiff_use_color_default = 1; /* bool */\n-\t\telse if (!strcasecmp(value, \"auto\")) {\n-\t\t\tdiff_use_color_default = 0;\n-\t\t\tif (isatty(1) || (pager_in_use && pager_use_color)) {\n-\t\t\t\tchar *term = getenv(\"TERM\");\n-\t\t\t\tif (term && strcmp(term, \"dumb\"))\n-\t\t\t\t\tdiff_use_color_default = 1;\n-\t\t\t}\n-\t\t}\n-\t\telse if (!strcasecmp(value, \"never\"))\n-\t\t\tdiff_use_color_default = 0;\n-\t\telse if (!strcasecmp(value, \"always\"))\n-\t\t\tdiff_use_color_default = 1;\n-\t\telse\n-\t\t\tdiff_use_color_default = git_config_bool(var, value);\n+\t\tdiff_use_color_default = git_config_colorbool(var, value);\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"diff.renames\")) {\n@@ -201,7 +73,7 @@ int git_diff_ui_config(const char *var, \n \t}\n \tif (!strncmp(var, \"diff.color.\", 11)) {\n \t\tint slot = parse_diff_color_slot(var, 11);\n-\t\tparse_diff_color_value(value, var, diff_colors[slot]);\n+\t\tcolor_parse(value, var, diff_colors[slot]);\n \t\treturn 0;\n \t}\n \treturn git_default_config(var, value);\n-- \n1.4.2.ge490e-dirty\n"},{"id":"26549","messageId":"7vfyf3s01w.fsf@assigned-by-dhcp.cox.net","threadId":"5504","inReplyTo":"20060907063559.GB17083@coredump.intra.peff.net","subject":"Re: [PATCH 2/3] Move color option parsing out of diff.c and into color.[ch]","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-07T23:56:43Z","receivedAt":"2006-09-07T23:56:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> +#include <stdarg.h>\n> +\n> +#define COLOR_RESET \"\\033[m\"\n> +\n> +static int\n> +parse_color(const char *name, int len)\n> +{\n\nStyle (applies to all functions you moved to this new file).\n\nSome school of programming teach us to start the function name\nat the beginning of the line, separate from its type.  They say\nthat would make \"grep '^parse_color'\" to work better.\n\nThat happens to be the way I was taught (actually it was a bit\nmore strange that return type was to be indented by one TAB, so\nit looked like this:\n\n\t\tstatic int\n\tparse_color(const char *name, int len)\n\n).  But git style is the kernel style, and I refrain from doing\nthat myself.\n\n\tstatic int parse_color(const char *name, int len)\n\t{\n        \t...\n\n> +int\n> +git_config_colorbool(const char *var, const char *value)\n> +{\n> +\tif (!value)\n> +\t\treturn 1;\n> +\tif (!strcasecmp(value, \"auto\")) {\n> +\t\tif (isatty(1) || (pager_in_use && pager_use_color)) {\n> +\t\t\tchar *term = getenv(\"TERM\");\n> +\t\t\tif (term && strcmp(term, \"dumb\"))\n> +\t\t\t\treturn 1;\n> +\t\t}\n> +\t\treturn 0;\n> +\t}\n> +\tif (!strcasecmp(value, \"never\"))\n> +\t\treturn 0;\n> +\tif (!strcasecmp(value, \"always\"))\n> +\t\treturn 1;\n> +\treturn git_config_bool(var, value);\n> +}\n\n> +int\n> +color_printf(const char *color, const char *fmt, ...) {\n\nStyle.\n\n\tint color_printf(const char *color, const char *fmt, ...)\n\t{\n\t\t...\n"},{"id":"26550","messageId":"20060908001132.GA18649@coredump.intra.peff.net","threadId":"5504","inReplyTo":"7vfyf3s01w.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/3] Move color option parsing out of diff.c and into color.[ch]","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2006-09-08T00:11:32Z","receivedAt":"2006-09-08T00:11:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 07, 2006 at 04:56:43PM -0700, Junio C Hamano wrote:\n\n> Some school of programming teach us to start the function name\n> at the beginning of the line, separate from its type.  They say\n> that would make \"grep '^parse_color'\" to work better.\n\nHmm. I don't like that style either, I was just using it because I\nthought that was the git style (which must have been caused by seeing\nsome other function(s) with the same style -- but grepping through, it\nlooks like there are very few functions using it). I'll fix the patches,\nbut will wait to resend until I hear other comments.\n\n-Peff\n"}]}