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

[Junio C Hamano <gitster@pobox.com>]

Paul Tan <pyokagan@gmail.com> writes:

>> +       /* 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);

That may be more obvious, but I wanted to reuse the existing
sign_off_header[]; there is no variant that has a leading "end of
previous line".

I actually think it is not an uncommon thing to ask "Give me the
first occurrence of the string NEEDLE that appears at the beginning
of lines in BUF", so the right way to address the "is it more
readable" question would probably be to have a helper function to do
just that, and use it here and other places that answer that
question by themselves due to lack of such a helper.