Re: [PATCH v3 08/11] sequencer.c: teach append_signoff to avoid adding a duplicate newline

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

 



Brandon Casey wrote:

> Teach append_signoff to detect whether a blank line exists at the position
> that the signed-off-by line will be added, and avoid adding an additional
> one if one already exists.  This is necessary to allow format-patch to add a
> s-o-b to a patch with no commit message without adding an extra newline.

I assume this means you're preserving historical behavior when adding
a sign-off to a commit with empty description (which is a good thing),
but what is that behavior?  Is it a deliberate choice or something
that developed by default?

[...]
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1118,11 +1118,15 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
>  	for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--)
>  		; /* do nothing */
>  
> -	if (i)
> -		has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);
> -
> -	if (!has_footer)
> -		strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
> +	if (msgbuf->buf[i] != '\n') {
> +		if (i)
> +			has_footer = has_conforming_footer(msgbuf, &sob,
> +					ignore_footer);
> +
> +		if (!has_footer)
> +			strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
> +					"\n", 1);
> +	}
>  
>  	if (has_footer != 3 && (!no_dup_sob || has_footer != 2))

This is too much nesting for my small mind to keep track of.  Am I
correct in imagining the effect is the same as the following?

	has_footer = has_conforming_footer(...)

	if (!has_footer && msgbuf->buf[i] != '\n')
		strbuf_splice(...);	/* add blank line */

	if (has_footer != 3 && ...
--
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]