On Thu, Jul 26, 2018 at 10:18 AM Han-Wen Nienhuys <hanwen@xxxxxxxxxx> wrote: > > Supported keywords are "error", "warning", "hint" and "success". Thanks for taking this upstream. :-) > > TODO: > * make the coloring optional? What variable to use? This is the natural extension of the topic merged at a56fb3dcc09 (Merge branch 'js/colored-push-errors', 2018-05-08), which was merged rather recently, so I'd think extending the color.transport option would be useful here. (cc'd the authors of that series) > * doc for the coloring option. > * how to test? I think the best way to get started with a test is to be inspired by 8301266afa4 (push: test to verify that push errors are colored, 2018-04-21) from that series. > Signed-off-by: Han-Wen Nienhuys <hanwen@xxxxxxxxxx> > Change-Id: I090412a1288bc2caef0916447e28c2d0199da47d We do not do Change-Ids in git upstream. :/ As different workflows have different edges, everyone has their own little script to deal with those. For example Brandon has https://github.com/bmwill/dotfiles/blob/master/bin/check-patch that contains a section to remove change ids # Remove Change-Id from patch sed -i "/Change-Id:/d" "$f" > +void emit_sideband(struct strbuf *dest, const char *src, int n) { Coding style: we start the brace in the next line for new functions (but not after if/while/for) Also I did not think hard enough in the internal review, as this function is not emitting to the sideband. that is solely done by the xwrite call in recv_sideband. So maybe prepare_sideband would be a better name? > + // NOSUBMIT - maybe use transport.color property? Yes, that would be my suggestion (note that we do not use // comments) > + int want_color = want_color_stderr(GIT_COLOR_AUTO); > + > + if (!want_color) { > + strbuf_add(dest, src, n); > + return; > + } > + > + 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}, > + {}, > + }; > + > + while (isspace(*src)) { > + strbuf_addch(dest, *src); > + src++; > + n--; > + } > + > + for (struct kwtable* p = keywords; p->keyword; p++) { > + int len = strlen(p->keyword); > + if (!strncasecmp(p->keyword, src, len) && !isalnum(src[len])) { > + strbuf_addstr(dest, p->color); > + strbuf_add(dest, src, len); > + strbuf_addstr(dest, GIT_COLOR_RESET); > + n -= len; > + src += len; > + break; > + } > + } > + > + strbuf_add(dest, src, n); > +} > + > > /* > * Receive multiplexed output stream over git native protocol. > @@ -48,8 +95,10 @@ int recv_sideband(const char *me, int in_stream, int out) > len--; > switch (band) { > case 3: > - strbuf_addf(&outbuf, "%s%s%s", outbuf.len ? "\n" : "", > - DISPLAY_PREFIX, buf + 1); > + strbuf_addf(&outbuf, "%s%s", outbuf.len ? "\n" : "", > + DISPLAY_PREFIX); > + emit_sideband(&outbuf, buf+1, len); > + > retval = SIDEBAND_REMOTE_ERROR; > break; > case 2: > @@ -69,20 +118,22 @@ int recv_sideband(const char *me, int in_stream, int out) > if (!outbuf.len) > strbuf_addstr(&outbuf, DISPLAY_PREFIX); > if (linelen > 0) { > - strbuf_addf(&outbuf, "%.*s%s%c", > - linelen, b, suffix, *brk); > - } else { > - strbuf_addch(&outbuf, *brk); > + emit_sideband(&outbuf, b, linelen); > + strbuf_addstr(&outbuf, suffix); > } > + > + strbuf_addch(&outbuf, *brk); > xwrite(2, outbuf.buf, outbuf.len); > strbuf_reset(&outbuf); > > b = brk + 1; > } > > - if (*b) > - strbuf_addf(&outbuf, "%s%s", outbuf.len ? > - "" : DISPLAY_PREFIX, b); > + if (*b) { > + strbuf_addstr(&outbuf, outbuf.len ? > + "" : DISPLAY_PREFIX); > + emit_sideband(&outbuf, b, strlen(b)); > + } > break; > case 1: > write_or_die(out, buf + 1, len); > -- > 2.18.0.233.g985f88cf7e-goog >
