Re: [PATCH] am: match --signoff to the original scripted version

From: Paul Tan <pyokagan@gmail.com>
Date: Sep 6, 2015, 14:21 UTC
Message-ID: <CACRoPnQvNowZGbqvRLuhS8cC1EaTQX3vsyaESSQVXpuRToWLOQ@mail.gmail.com>
In-Reply-To: <xmqqio7ob0xw.fsf@gitster.mtv.corp.google.com>

On Sun, Sep 6, 2015 at 12:56 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 31 quoted lines

> diff --git a/builtin/am.c b/builtin/am.c
> index 634f7a7..e7828e5 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -1191,6 +1191,33 @@ static void NORETURN die_user_resolve(const struct am_state *state)
>         exit(128);
>  }
>
> +static void am_signoff(struct strbuf *sb)
> +{
> +       char *cp;
> +       struct strbuf mine = STRBUF_INIT;
> +
> +       /* Does it end with our own sign-off? */
> +       strbuf_addf(&mine, "\n%s%s\n",
> +                   sign_off_header,
> +                   fmt_name(getenv("GIT_COMMITTER_NAME"),
> +                            getenv("GIT_COMMITTER_EMAIL")));
> +       if (mine.len < sb->len &&
> +           !strcmp(mine.buf, sb->buf + sb->len - mine.len))
> +               goto exit; /* no need to duplicate */
> +
> +       /* Does it have any Signed-off-by: in the text */
> +       for (cp = sb->buf;
> +            cp && *cp && (cp = strstr(cp, sign_off_header)) != NULL;
> +            cp = strchr(cp, '\n')) {
> +               if (sb->buf == cp || cp[-1] == '\n')
> +                       break;
> +       }
> +
> +       strbuf_addstr(sb, mine.buf + !!cp);

To add on, I wonder if the above "add a blank line if there is no
Signed-off-by: at the beginning of a line" logic could be expressed
more succinctly like this:

    if (!starts_with(sb->buf, "Signed-off-by: ") &&
        !strstr(sb->buf, "\nSigned-off-by: "))
        strbuf_addch(sb, '\n');

    strbuf_addstr(sb, mine.buf + 1);

Show 7 quoted lines

> +exit:
> +       strbuf_release(&mine);
> +}
> +
>  /**
>   * Appends signoff to the "msg" field of the am_state.
>   */

Regards, Paul