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

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

 



Paul Tan <pyokagan@xxxxxxxxx> 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.



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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]