On Tue, Jul 31, 2018 at 09:37:51AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
> On Mon, Jul 30 2018, Han-Wen Nienhuys wrote:
>
>
> > + 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< --
