Re: [PATCH] RFC Highlight keywords in remote sideband output.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux