From 0edad17d6728df055c3b92bf89d5329f0afdcefd Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Nov 2014 10:15:31 -0500 Subject: [PATCH 1/3] docs: describe ANSI 256-color mode Our color specifications have supported the 256-color ANSI extension for years, but we never documented it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/config.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 6ae4d90708..9678ab6aa8 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -825,6 +825,10 @@ accepted are `normal`, `black`, `red`, `green`, `yellow`, `blue`, `blink` and `reverse`. The first color given is the foreground; the second is the background. The position of the attribute, if any, doesn't matter. ++ +Colors (foreground and background) may also be given as numbers between +0 and 255; these use ANSI 256-color mode (but note that not all +terminals may support this). color.diff:: Whether to use ANSI escape sequences to add color to patches. From d0e08d62335ce9685ac547287771e589587334b6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Nov 2014 10:15:51 -0500 Subject: [PATCH 2/3] config: fix parsing of "git config --get-color some.key -1" Most of git-config's command line options use OPT_BIT to choose an action, and then parse the non-option arguments in a context-dependent way. However, --get-color and --get-colorbool are unlike the rest of the options, in that they are OPT_STRING, taking the option name as a parameter. This generally works, because we then use the presence of those strings to set an action bit anyway. But it does mean that the option-parser will continue looking for options even after the key (because it is not a non-option; it is an argument to an option). And running: git config --get-color some.key -1 (to use "-1" as the default color spec) will barf, claiming that "-1" is not an option. Instead, we should treat --get-color and --get-colorbool as action bits, just like --add, --get, and all the other actions, and then check that the non-option arguments we got are sane. This fixes the weirdness above, and makes those two options like all the others. This "fixes" a test in t4026, which checked that feeding "-2" as a color should fail (it does fail, but prior to this patch, because parseopt barfed, not because we actually ever tried to parse the color). This also catches other errors, like: git config --get-color some.key black blue which previously silently ignored "blue" (and now will complain that you gave too many arguments). There are some possible regressions, though. We now disallow these, which currently do what you would expect: # specifying other options after the action git config --get-color some.key --file whatever # using long-arg syntax git config --get-color=some.key However, we have never advertised these in the documentation, and in fact they did not work in some older versions of git. The behavior was apparently switched as an accidental side effect of d64ec16 (git config: reorganize to use parseopt, 2009-02-21). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/config.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index 7bba516383..84b8e1cfa5 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -69,8 +69,8 @@ static struct option builtin_config_options[] = { OPT_BIT(0, "remove-section", &actions, N_("remove a section: name"), ACTION_REMOVE_SECTION), OPT_BIT('l', "list", &actions, N_("list all"), ACTION_LIST), OPT_BIT('e', "edit", &actions, N_("open an editor"), ACTION_EDIT), - OPT_STRING(0, "get-color", &get_color_slot, N_("slot"), N_("find the color configured: [default]")), - OPT_STRING(0, "get-colorbool", &get_colorbool_slot, N_("slot"), N_("find the color setting: [stdout-is-tty]")), + OPT_BIT(0, "get-color", &actions, N_("find the color configured: slot [default]"), ACTION_GET_COLOR), + OPT_BIT(0, "get-colorbool", &actions, N_("find the color setting: slot [stdout-is-tty]"), ACTION_GET_COLORBOOL), OPT_GROUP(N_("Type")), OPT_BIT(0, "bool", &types, N_("value is \"true\" or \"false\""), TYPE_BOOL), OPT_BIT(0, "int", &types, N_("value is decimal number"), TYPE_INT), @@ -302,8 +302,9 @@ static int git_get_color_config(const char *var, const char *value, void *cb) return 0; } -static void get_color(const char *def_color) +static void get_color(const char *var, const char *def_color) { + get_color_slot = var; get_color_found = 0; parsed_color[0] = '\0'; git_config_with_options(git_get_color_config, NULL, @@ -330,8 +331,9 @@ static int git_get_colorbool_config(const char *var, const char *value, return 0; } -static int get_colorbool(int print) +static int get_colorbool(const char *var, int print) { + get_colorbool_slot = var; get_colorbool_found = -1; get_diff_color_found = -1; get_color_ui_found = -1; @@ -515,12 +517,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) usage_with_options(builtin_config_usage, builtin_config_options); } - if (get_color_slot) - actions |= ACTION_GET_COLOR; - if (get_colorbool_slot) - actions |= ACTION_GET_COLORBOOL; - - if ((get_color_slot || get_colorbool_slot) && types) { + if ((actions & (ACTION_GET_COLOR|ACTION_GET_COLORBOOL)) && types) { error("--get-color and variable type are incoherent"); usage_with_options(builtin_config_usage, builtin_config_options); } @@ -655,12 +652,14 @@ int cmd_config(int argc, const char **argv, const char *prefix) die("No such section!"); } else if (actions == ACTION_GET_COLOR) { - get_color(argv[0]); + check_argc(argc, 1, 2); + get_color(argv[0], argv[1]); } else if (actions == ACTION_GET_COLORBOOL) { - if (argc == 1) - color_stdout_is_tty = git_config_bool("command line", argv[0]); - return get_colorbool(argc != 0); + check_argc(argc, 1, 2); + if (argc == 2) + color_stdout_is_tty = git_config_bool("command line", argv[1]); + return get_colorbool(argv[0], argc == 2); } return 0; From cb357221a402c55b23bf99ec9f3b361709d45fa7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Nov 2014 10:16:09 -0500 Subject: [PATCH 3/3] t4026: test "normal" color If the user specifiers "normal" for a foreground color, this should be a noop (while this may sound useless, it is the only way to specify an unchanged foreground color followed by a specific background color). We also check that color "-1" does the same thing. This is not documented, but has worked forever, so let's make sure we keep supporting it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t4026-color.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/t/t4026-color.sh b/t/t4026-color.sh index 3726a0e201..63e423838f 100755 --- a/t/t4026-color.sh +++ b/t/t4026-color.sh @@ -53,6 +53,14 @@ test_expect_success '256 colors' ' color "254 bold 255" "[1;38;5;254;48;5;255m" ' +test_expect_success '"normal" yields no color at all"' ' + color "normal black" "[40m" +' + +test_expect_success '-1 is a synonym for "normal"' ' + color "-1 black" "[40m" +' + test_expect_success 'color too small' ' invalid_color "-2" '