Re: [PATCH 1/1] Highlight keywords in remote sideband output.
From: Duy Nguyen <hidden>
Date: 2018-07-31 15:06:55
Subsystem:
documentation, the rest · Maintainers:
Jonathan Corbet, Linus Torvalds
On Tue, Jul 31, 2018 at 09:37:51AM +0200, Ævar Arnfjörð Bjarmason wrote:
On Mon, Jul 30 2018, Han-Wen Nienhuys wrote:quoted
+ if (sideband_use_color < 0) { + const char *key = "color.remote"; + char *value = NULL; + if (!git_config_get_string(key, &value)) + sideband_use_color = git_config_colorbool(key, value); [...] + struct kwtable { + const char *keyword; + const char *color; + } keywords[] = { + {"hint", GIT_COLOR_YELLOW}, + {"warning", GIT_COLOR_BOLD_YELLOW}, + {"success", GIT_COLOR_BOLD_GREEN}, + {"error", GIT_COLOR_BOLD_RED},FWIW I agree with other reviewers that it would be nice if these could be customized, but I also think it can wait for some follow-up patch. Users who don't like these colors don't have to use them, and then they're no worse off than now, whereas some users will appreciate these and be better off than now. So great if you want to improve this, but just chiming in on that point because I think we should be respectful of the time of contributors, and not make perfect the enemy of the good.
Fair enough. I'll scratch my own itch. Can we squash this in then? -- 8< --
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 43b2de7b5f..0783323bec 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt@@ -1229,6 +1229,15 @@ color.push:: color.push.error:: Use customized color for push errors. +color.remote:: + A boolean to enable/disable colored remote output. If unset, + then the value of `color.ui` is used (`auto` by default). + +color.remote.<slot>:: + Use customized color for each remote keywords. `<slot>` may be + `hint`, `warning`, `success` or `error` which match the + corresponding keyword. + color.showBranch:: A boolean to enable/disable color in the output of linkgit:git-show-branch[1]. May be set to `always`,
diff --git a/help.c b/help.c
index 3ebf0568db..b6cafcfc0a 100644
--- a/help.c
+++ b/help.c@@ -425,6 +425,7 @@ void list_config_help(int for_human) { "color.diff", "<slot>", list_config_color_diff_slots }, { "color.grep", "<slot>", list_config_color_grep_slots }, { "color.interactive", "<slot>", list_config_color_interactive_slots }, + { "color.remote", "<slot>", list_config_color_sideband_slots }, { "color.status", "<slot>", list_config_color_status_slots }, { "fsck", "<msg-id>", list_config_fsck_msg_ids }, { "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
diff --git a/help.h b/help.h
index f8b15323a6..9eab6a3f89 100644
--- a/help.h
+++ b/help.h@@ -83,6 +83,7 @@ void list_config_color_diff_slots(struct string_list *list, const char *prefix); void list_config_color_grep_slots(struct string_list *list, const char *prefix); void list_config_color_interactive_slots(struct string_list *list, const char *prefix); void list_config_color_status_slots(struct string_list *list, const char *prefix); +void list_config_color_sideband_slots(struct string_list *list, const char *prefix); void list_config_fsck_msg_ids(struct string_list *list, const char *prefix); #endif /* HELP_H */
diff --git a/sideband.c b/sideband.c
index 74b2fcaf64..61c4376a64 100644
--- a/sideband.c
+++ b/sideband.c@@ -3,7 +3,59 @@ #include "config.h" #include "pkt-line.h" #include "sideband.h" +#include "help.h" + +struct kwtable { + /* + * Note, we current use keyword as config key so it can't + * contain funny chars like spaces. When we do that, we need a + * separate field for slot name in load_sideband_colors(). + */ + const char *keyword; + char color[COLOR_MAXLEN]; +}; + +static struct kwtable keywords[] = { + { "hint", GIT_COLOR_YELLOW }, + { "warning", GIT_COLOR_BOLD_YELLOW }, + { "success", GIT_COLOR_BOLD_GREEN }, + { "error", GIT_COLOR_BOLD_RED }, +}; + +static int sideband_use_color = -1; + +static void load_sideband_colors(void) +{ + const char *key = "color.remote"; + struct strbuf sb = STRBUF_INIT; + char *value; + int i; + + if (sideband_use_color >= 0) + return; + + if (!git_config_get_string(key, &value)) + sideband_use_color = git_config_colorbool(key, value); + + for (i = 0; i < ARRAY_SIZE(keywords); i++) { + strbuf_reset(&sb); + strbuf_addf(&sb, "%s.%s", key, keywords[i].keyword); + if (git_config_get_string(sb.buf, &value)) + continue; + + if (color_parse(value, keywords[i].color)) + die(_("expecting a color: %s"), value); + } + strbuf_release(&sb); +} + +void list_config_color_sideband_slots(struct string_list *list, const char *prefix) +{ + int i; + for (i = 0; i < ARRAY_SIZE(keywords); i++) + list_config_item(list, prefix, keywords[i].keyword); +} /* * Optionally highlight some keywords in remote output if they appear at the
@@ -11,24 +63,9 @@ */ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n) { - static int sideband_use_color = -1; int i; - struct kwtable { - const char *keyword; - const char *color; - } keywords[] = { - {"hint", GIT_COLOR_YELLOW}, - {"warning", GIT_COLOR_BOLD_YELLOW}, - {"success", GIT_COLOR_BOLD_GREEN}, - {"error", GIT_COLOR_BOLD_RED}, - }; - - if (sideband_use_color < 0) { - const char *key = "color.remote"; - char *value = NULL; - if (!git_config_get_string(key, &value)) - sideband_use_color = git_config_colorbool(key, value); - } + + load_sideband_colors(); if (!want_color_stderr(sideband_use_color)) { strbuf_add(dest, src, n);
diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh
index 606386f4fe..69b7cc571d 100755
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh@@ -6,7 +6,7 @@ test_description='remote messages are colorized on the client' test_expect_success 'setup' ' mkdir .git/hooks && - cat << EOF > .git/hooks/update && + cat << EOF >.git/hooks/update && #!/bin/sh echo error: error echo hint: hint
@@ -20,15 +20,26 @@ EOF git commit -m 1 && git clone . child && cd child && - echo 2 > file && + echo 2 >file && git commit -a -m 2 ' -test_expect_success 'push' 'git -c color.remote=always push origin HEAD:refs/heads/newbranch 2>output && - test_decode_color < output > decoded && - test_i18ngrep "<BOLD;RED>error<RESET>:" decoded && - test_i18ngrep "<YELLOW>hint<RESET>:" decoded && - test_i18ngrep "<BOLD;GREEN>success<RESET>:" decoded && - test_i18ngrep "<BOLD;YELLOW>warning<RESET>:" decoded' +test_expect_success 'push' ' + git -c color.remote=always push origin HEAD:refs/heads/newbranch 2>output && + test_decode_color <output >decoded && + test_i18ngrep "<BOLD;RED>error<RESET>:" decoded && + test_i18ngrep "<YELLOW>hint<RESET>:" decoded && + test_i18ngrep "<BOLD;GREEN>success<RESET>:" decoded && + test_i18ngrep "<BOLD;YELLOW>warning<RESET>:" decoded +' + +test_expect_success 'push with customized color' ' + git -c color.remote=always -c color.remote.error=white push origin HEAD:refs/heads/newbranch2 2>output && + test_decode_color <output >decoded && + test_i18ngrep "<WHITE>error<RESET>:" decoded && + test_i18ngrep "<YELLOW>hint<RESET>:" decoded && + test_i18ngrep "<BOLD;GREEN>success<RESET>:" decoded && + test_i18ngrep "<BOLD;YELLOW>warning<RESET>:" decoded +' test_done -- 8< --