Re: [PATCH v3 05/11] sequencer.c: recognize (cherry picked from ... as part of s-o-b footer

2013-01-27 Thread Jonathan Nieder 
Brandon Casey wrote:

 Let's detect (cherry picked from...) as part of the footer so that we
 will produce this:

Signed-off-by: A U Thor aut...@example.com
(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)
Signed-off-by: C O Mmitter commit...@example.com

 instead of this:

Signed-off-by: A U Thor aut...@example.com
(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)

Signed-off-by: C O Mmitter commit...@example.com

Here's the tweak I suggested last time.  I think its behavior is
slightly better in the ends with incomplete line case because it
limits the characters examined by is_rfc2822_line() and
is_cherry_picked_from_line() not to include buf[len] (which would
presumably sometimes be '\0').

diff --git i/sequencer.c w/sequencer.c
index 0b5cd18c..fa29c7c5 100644
--- i/sequencer.c
+++ w/sequencer.c
@@ -1043,9 +1043,7 @@ static int is_cherry_picked_from_line(const char *buf, 
int len)
/*
 * We only care that it looks roughly like (cherry picked from ...)
 */
-   return !prefixcmp(buf, cherry_picked_prefix) 
-   (buf[len - 1] == ')' ||
-(buf[len - 1] == '\n'  buf[len - 2] == ')'));
+   return !prefixcmp(buf, cherry_picked_prefix)  buf[len - 1] == ')';
 }
 
 static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
@@ -1072,8 +1070,8 @@ static int has_conforming_footer(struct strbuf *sb, int 
ignore_footer)
; /* do nothing */
k++;
 
-   if (!(is_rfc2822_line(buf + i, k - i) ||
-   is_cherry_picked_from_line(buf + i, k - i)))
+   if (!is_rfc2822_line(buf + i, k - i - 1) 
+   !is_cherry_picked_from_line(buf + i, k - i - 1))
return 0;
}
return 1;
--
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

Re: [PATCH v3 05/11] sequencer.c: recognize (cherry picked from ... as part of s-o-b footer

2013-01-27 Thread Jonathan Nieder 
Jonathan Nieder wrote:

 Here's the tweak I suggested last time.  I think its behavior is
 slightly better in the ends with incomplete line case because it
 limits the characters examined by is_rfc2822_line() and
 is_cherry_picked_from_line() not to include buf[len] (which would
 presumably sometimes be '\0').

Whoops, that revealed a subtlety --- the '\n' or '\0' is what prevents
exiting the loop in is_rfc2822_line when the line does not contain a
colon.  Here's a corrected version of the tweak, that should actually
work. :)

diff --git i/sequencer.c w/sequencer.c
index 0b5cd18c..108ea27b 100644
--- i/sequencer.c
+++ w/sequencer.c
@@ -1029,13 +1029,11 @@ static int is_rfc2822_line(const char *buf, int len)
for (i = 0; i  len; i++) {
int ch = buf[i];
if (ch == ':')
+   return 1;
+   if (!isalnum(ch)  ch != '-')
break;
-   if (isalnum(ch) || (ch == '-'))
-   continue;
-   return 0;
}
-
-   return 1;
+   return 0;
 }
 
 static int is_cherry_picked_from_line(const char *buf, int len)
@@ -1043,9 +1041,7 @@ static int is_cherry_picked_from_line(const char *buf, 
int len)
/*
 * We only care that it looks roughly like (cherry picked from ...)
 */
-   return !prefixcmp(buf, cherry_picked_prefix) 
-   (buf[len - 1] == ')' ||
-(buf[len - 1] == '\n'  buf[len - 2] == ')'));
+   return !prefixcmp(buf, cherry_picked_prefix)  buf[len - 1] == ')';
 }
 
 static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
@@ -1072,8 +1068,8 @@ static int has_conforming_footer(struct strbuf *sb, int 
ignore_footer)
; /* do nothing */
k++;
 
-   if (!(is_rfc2822_line(buf + i, k - i) ||
-   is_cherry_picked_from_line(buf + i, k - i)))
+   if (!is_rfc2822_line(buf + i, k - i - 1) 
+   !is_cherry_picked_from_line(buf + i, k - i - 1))
return 0;
}
return 1;
--
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

Re: [PATCH v3 05/11] sequencer.c: recognize (cherry picked from ... as part of s-o-b footer

2013-01-27 Thread Brandon Casey 
On Sun, Jan 27, 2013 at 6:51 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Jonathan Nieder wrote:

 Here's the tweak I suggested last time.  I think its behavior is
 slightly better in the ends with incomplete line case because it
 limits the characters examined by is_rfc2822_line() and
 is_cherry_picked_from_line() not to include buf[len] (which would
 presumably sometimes be '\0').

 Whoops, that revealed a subtlety --- the '\n' or '\0' is what prevents
 exiting the loop in is_rfc2822_line when the line does not contain a
 colon.  Here's a corrected version of the tweak, that should actually
 work. :)

 diff --git i/sequencer.c w/sequencer.c
 index 0b5cd18c..108ea27b 100644
 --- i/sequencer.c
 +++ w/sequencer.c
 @@ -1029,13 +1029,11 @@ static int is_rfc2822_line(const char *buf, int len)
 for (i = 0; i  len; i++) {
 int ch = buf[i];
 if (ch == ':')
 +   return 1;
 +   if (!isalnum(ch)  ch != '-')
 break;
 -   if (isalnum(ch) || (ch == '-'))
 -   continue;
 -   return 0;
 }
 -
 -   return 1;
 +   return 0;
  }

  static int is_cherry_picked_from_line(const char *buf, int len)
 @@ -1043,9 +1041,7 @@ static int is_cherry_picked_from_line(const char *buf, 
 int len)
 /*
  * We only care that it looks roughly like (cherry picked from ...)
  */
 -   return !prefixcmp(buf, cherry_picked_prefix) 
 -   (buf[len - 1] == ')' ||
 -(buf[len - 1] == '\n'  buf[len - 2] == ')'));
 +   return !prefixcmp(buf, cherry_picked_prefix)  buf[len - 1] == ')';
  }

  static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
 @@ -1072,8 +1068,8 @@ static int has_conforming_footer(struct strbuf *sb, int 
 ignore_footer)
 ; /* do nothing */
 k++;

 -   if (!(is_rfc2822_line(buf + i, k - i) ||
 -   is_cherry_picked_from_line(buf + i, k - i)))
 +   if (!is_rfc2822_line(buf + i, k - i - 1) 
 +   !is_cherry_picked_from_line(buf + i, k - i - 1))
 return 0;
 }
 return 1;

Looks good to me.

-Brandon
--
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