Skip to content

Commit bf1a11f

Browse files
hanwengitster
hanwen
authored and
gitster
committed
sideband: highlight keywords in remote sideband output
The colorization is controlled with the config setting "color.remote". Supported keywords are "error", "warning", "hint" and "success". They are highlighted if they appear at the start of the line, which is common in error messages, eg. ERROR: commit is missing Change-Id The Git push process itself prints lots of non-actionable messages (eg. bandwidth statistics, object counters for different phases of the process). This obscures actionable error messages that servers may send back. Highlighting keywords in the sideband draws more attention to those messages. The background for this change is that Gerrit does server-side processing to create or update code reviews, and actionable error messages (eg. missing Change-Id) must be communicated back to the user during the push. User research has shown that new users have trouble seeing these messages. The highlighting is done on the client rather than server side, so servers don't have to grow capabilities to understand terminal escape codes and terminal state. It also consistent with the current state where Git is control of the local display (eg. prefixing messages with "remote: "). The highlighting can be configured using color.remote.<KEYWORD> configuration settings. Since the keys are matched case insensitively, we match the keywords case insensitively too. Finally, this solution is backwards compatible: many servers already prefix their messages with "error", and they will benefit from this change without requiring a server update. By contrast, a server-side solution would likely require plumbing the TERM variable through the git protocol, so it would require changes to both server and client. Helped-by: Duy Nguyen <[email protected]> Signed-off-by: Han-Wen Nienhuys <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 1d89318 commit bf1a11f

File tree

5 files changed

+217
-9
lines changed

5 files changed

+217
-9
lines changed

‎Documentation/config.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,6 +1263,18 @@ color.push::
12631263
color.push.error::
12641264
Use customized color for push errors.
12651265

1266+
color.remote::
1267+
If set, keywords at the start of the line are highlighted. The
1268+
keywords are "error", "warning", "hint" and "success", and are
1269+
matched case-insensitively. May be set to `always`, `false` (or
1270+
`never`) or `auto` (or `true`). If unset, then the value of
1271+
`color.ui` is used (`auto` by default).
1272+
1273+
color.remote.<slot>::
1274+
Use customized color for each remote keyword. `<slot>` may be
1275+
`hint`, `warning`, `success` or `error` which match the
1276+
corresponding keyword.
1277+
12661278
color.showBranch::
12671279
A boolean to enable/disable color in the output of
12681280
linkgit:git-show-branch[1]. May be set to `always`,

‎help.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,7 @@ void list_config_help(int for_human)
425425
{ "color.diff", "<slot>", list_config_color_diff_slots },
426426
{ "color.grep", "<slot>", list_config_color_grep_slots },
427427
{ "color.interactive", "<slot>", list_config_color_interactive_slots },
428+
{ "color.remote", "<slot>", list_config_color_sideband_slots },
428429
{ "color.status", "<slot>", list_config_color_status_slots },
429430
{ "fsck", "<msg-id>", list_config_fsck_msg_ids },
430431
{ "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },

‎help.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ void list_config_color_diff_slots(struct string_list *list, const char *prefix);
8383
void list_config_color_grep_slots(struct string_list *list, const char *prefix);
8484
void list_config_color_interactive_slots(struct string_list *list, const char *prefix);
8585
void list_config_color_status_slots(struct string_list *list, const char *prefix);
86+
void list_config_color_sideband_slots(struct string_list *list, const char *prefix);
8687
void list_config_fsck_msg_ids(struct string_list *list, const char *prefix);
8788

8889
#endif /* HELP_H */

‎sideband.c

Lines changed: 116 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,109 @@
11
#include "cache.h"
2+
#include "color.h"
3+
#include "config.h"
24
#include "pkt-line.h"
35
#include "sideband.h"
6+
#include "help.h"
7+
8+
struct keyword_entry {
9+
/*
10+
* We use keyword as config key so it should be a single alphanumeric word.
11+
*/
12+
const char *keyword;
13+
char color[COLOR_MAXLEN];
14+
};
15+
16+
static struct keyword_entry keywords[] = {
17+
{ "hint", GIT_COLOR_YELLOW },
18+
{ "warning", GIT_COLOR_BOLD_YELLOW },
19+
{ "success", GIT_COLOR_BOLD_GREEN },
20+
{ "error", GIT_COLOR_BOLD_RED },
21+
};
22+
23+
/* Returns a color setting (GIT_COLOR_NEVER, etc). */
24+
static int use_sideband_colors(void)
25+
{
26+
static int use_sideband_colors_cached = -1;
27+
28+
const char *key = "color.remote";
29+
struct strbuf sb = STRBUF_INIT;
30+
char *value;
31+
int i;
32+
33+
if (use_sideband_colors_cached >= 0)
34+
return use_sideband_colors_cached;
35+
36+
if (!git_config_get_string(key, &value)) {
37+
use_sideband_colors_cached = git_config_colorbool(key, value);
38+
} else if (!git_config_get_string("color.ui", &value)) {
39+
use_sideband_colors_cached = git_config_colorbool("color.ui", value);
40+
} else {
41+
use_sideband_colors_cached = GIT_COLOR_AUTO;
42+
}
43+
44+
for (i = 0; i < ARRAY_SIZE(keywords); i++) {
45+
strbuf_reset(&sb);
46+
strbuf_addf(&sb, "%s.%s", key, keywords[i].keyword);
47+
if (git_config_get_string(sb.buf, &value))
48+
continue;
49+
if (color_parse(value, keywords[i].color))
50+
continue;
51+
}
52+
strbuf_release(&sb);
53+
return use_sideband_colors_cached;
54+
}
55+
56+
void list_config_color_sideband_slots(struct string_list *list, const char *prefix)
57+
{
58+
int i;
59+
60+
for (i = 0; i < ARRAY_SIZE(keywords); i++)
61+
list_config_item(list, prefix, keywords[i].keyword);
62+
}
63+
64+
/*
65+
* Optionally highlight one keyword in remote output if it appears at the start
66+
* of the line. This should be called for a single line only, which is
67+
* passed as the first N characters of the SRC array.
68+
*/
69+
static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
70+
{
71+
int i;
72+
73+
if (!want_color_stderr(use_sideband_colors())) {
74+
strbuf_add(dest, src, n);
75+
return;
76+
}
77+
78+
while (isspace(*src)) {
79+
strbuf_addch(dest, *src);
80+
src++;
81+
n--;
82+
}
83+
84+
for (i = 0; i < ARRAY_SIZE(keywords); i++) {
85+
struct keyword_entry *p = keywords + i;
86+
int len = strlen(p->keyword);
87+
/*
88+
* Match case insensitively, so we colorize output from existing
89+
* servers regardless of the case that they use for their
90+
* messages. We only highlight the word precisely, so
91+
* "successful" stays uncolored.
92+
*/
93+
if (!strncasecmp(p->keyword, src, len) && !isalnum(src[len])) {
94+
strbuf_addstr(dest, p->color);
95+
strbuf_add(dest, src, len);
96+
strbuf_addstr(dest, GIT_COLOR_RESET);
97+
n -= len;
98+
src += len;
99+
break;
100+
}
101+
}
102+
103+
strbuf_add(dest, src, n);
104+
105+
}
106+
4107

5108
/*
6109
* Receive multiplexed output stream over git native protocol.
@@ -48,8 +151,10 @@ int recv_sideband(const char *me, int in_stream, int out)
48151
len--;
49152
switch (band) {
50153
case 3:
51-
strbuf_addf(&outbuf, "%s%s%s", outbuf.len ? "\n" : "",
52-
DISPLAY_PREFIX, buf + 1);
154+
strbuf_addf(&outbuf, "%s%s", outbuf.len ? "\n" : "",
155+
DISPLAY_PREFIX);
156+
maybe_colorize_sideband(&outbuf, buf + 1, len);
157+
53158
retval = SIDEBAND_REMOTE_ERROR;
54159
break;
55160
case 2:
@@ -69,20 +174,22 @@ int recv_sideband(const char *me, int in_stream, int out)
69174
if (!outbuf.len)
70175
strbuf_addstr(&outbuf, DISPLAY_PREFIX);
71176
if (linelen > 0) {
72-
strbuf_addf(&outbuf, "%.*s%s%c",
73-
linelen, b, suffix, *brk);
74-
} else {
75-
strbuf_addch(&outbuf, *brk);
177+
maybe_colorize_sideband(&outbuf, b, linelen);
178+
strbuf_addstr(&outbuf, suffix);
76179
}
180+
181+
strbuf_addch(&outbuf, *brk);
77182
xwrite(2, outbuf.buf, outbuf.len);
78183
strbuf_reset(&outbuf);
79184

80185
b = brk + 1;
81186
}
82187

83-
if (*b)
84-
strbuf_addf(&outbuf, "%s%s", outbuf.len ?
85-
"" : DISPLAY_PREFIX, b);
188+
if (*b) {
189+
strbuf_addstr(&outbuf, outbuf.len ?
190+
"" : DISPLAY_PREFIX);
191+
maybe_colorize_sideband(&outbuf, b, strlen(b));
192+
}
86193
break;
87194
case 1:
88195
write_or_die(out, buf + 1, len);

‎t/t5409-colorize-remote-messages.sh

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
#!/bin/sh
2+
3+
test_description='remote messages are colorized on the client'
4+
5+
. ./test-lib.sh
6+
7+
test_expect_success 'setup' '
8+
mkdir .git/hooks &&
9+
write_script .git/hooks/update <<-\EOF &&
10+
echo error: error
11+
echo ERROR: also highlighted
12+
echo hint: hint
13+
echo hinting: not highlighted
14+
echo success: success
15+
echo warning: warning
16+
echo prefixerror: error
17+
echo " " "error: leading space"
18+
exit 0
19+
EOF
20+
echo 1 >file &&
21+
git add file &&
22+
git commit -m 1 &&
23+
git clone . child &&
24+
(
25+
cd child &&
26+
test_commit message2 file content2
27+
)
28+
'
29+
30+
test_expect_success 'keywords' '
31+
git --git-dir child/.git -c color.remote=always push -f origin HEAD:refs/heads/keywords 2>output &&
32+
test_decode_color <output >decoded &&
33+
grep "<BOLD;RED>error<RESET>: error" decoded &&
34+
grep "<YELLOW>hint<RESET>:" decoded &&
35+
grep "<BOLD;GREEN>success<RESET>:" decoded &&
36+
grep "<BOLD;YELLOW>warning<RESET>:" decoded
37+
'
38+
39+
test_expect_success 'whole words at line start' '
40+
git --git-dir child/.git -c color.remote=always push -f origin HEAD:refs/heads/whole-words 2>output &&
41+
test_decode_color <output >decoded &&
42+
grep "<YELLOW>hint<RESET>:" decoded &&
43+
grep "hinting: not highlighted" decoded &&
44+
grep "prefixerror: error" decoded
45+
'
46+
47+
test_expect_success 'case-insensitive' '
48+
git --git-dir child/.git -c color.remote=always push -f origin HEAD:refs/heads/case-insensitive 2>output &&
49+
cat output &&
50+
test_decode_color <output >decoded &&
51+
grep "<BOLD;RED>error<RESET>: error" decoded &&
52+
grep "<BOLD;RED>ERROR<RESET>: also highlighted" decoded
53+
'
54+
55+
test_expect_success 'leading space' '
56+
git --git-dir child/.git -c color.remote=always push -f origin HEAD:refs/heads/leading-space 2>output && cat output &&
57+
test_decode_color <output >decoded &&
58+
grep " <BOLD;RED>error<RESET>: leading space" decoded
59+
'
60+
61+
test_expect_success 'no coloring for redirected output' '
62+
git --git-dir child/.git push -f origin HEAD:refs/heads/redirected-output 2>output &&
63+
test_decode_color <output >decoded &&
64+
grep "error: error" decoded
65+
'
66+
67+
test_expect_success 'push with customized color' '
68+
git --git-dir child/.git -c color.remote=always -c color.remote.error=blue push -f origin HEAD:refs/heads/customized-color 2>output &&
69+
test_decode_color <output >decoded &&
70+
grep "<BLUE>error<RESET>:" decoded &&
71+
grep "<BOLD;GREEN>success<RESET>:" decoded
72+
'
73+
74+
75+
test_expect_success 'error in customized color' '
76+
git --git-dir child/.git -c color.remote=always -c color.remote.error=i-am-not-a-color push -f origin HEAD:refs/heads/error-customized-color 2>output &&
77+
test_decode_color <output >decoded &&
78+
grep "<BOLD;GREEN>success<RESET>:" decoded
79+
'
80+
81+
test_expect_success 'fallback to color.ui' '
82+
git --git-dir child/.git -c color.ui=always push -f origin HEAD:refs/heads/fallback-color-ui 2>output &&
83+
test_decode_color <output >decoded &&
84+
grep "<BOLD;RED>error<RESET>: error" decoded
85+
'
86+
87+
test_done

0 commit comments

Comments
 (0)