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

2013-01-27 Thread Jonathan Nieder
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-01-27 Thread Brandon Casey
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.  A
following patch will make format-patch use this function rather than the
append_signoff implementation inside log-tree.c.

Signed-off-by: Brandon Casey 
---
 sequencer.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 015cb58..404b786 100644
--- 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))
strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
-- 
1.8.1.1.450.g0327af3

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