Re: [PATCH] RFC Highlight keywords in remote sideband output.
- From
Stefan Beller <sbeller@google.com>
- Date
- Jul 26, 2018, 17:53 UTC
- Message-ID
- <CAGZ79kaj3ptbSrWfrZhRjcz1fsCmO_sR5qCfL=F1ZYwwJRBaMA@mail.gmail.com>
- In-Reply-To
- <20180726171751.178616-1-hanwen@google.com>
On Thu, Jul 26, 2018 at 10:18 AM Han-Wen Nienhuys <hanwen@google.com> 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@google.com> > 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)
Show 88 quoted lines
> + 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
>