Re: [PATCH] trailer: ignore first line of message

2015-08-21 Thread Matthieu Moy
Christian Couder christian.cou...@gmail.com writes:

 When looking for the start of the trailers in the message
 we are passed, we should ignore the first line of the message.

Thanks, this fixes my issue.

There's one more corner-case I've just thought of:

git commit -m 'place of
code: change we made'

(with the line break)

Git considers this message as a summary line broken in the middle. git
log --oneline shows it as a one-liner, as if it were
'place of code: change we made'.

Even with your patch, the trailer is added without a blank line, and
renders on the subject line in `git log --oneline`. My command above
with a commit-msg hook outputs:

[master 86f32d5] place of: code: change we made Signed-off-by: Matthieu Moy 
matthieu@imag.fr

(on a single line)

I do not care deeply, but you may want to let interpret-trailers deal
with this case too.

Thanks,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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] trailer: ignore first line of message

2015-08-20 Thread Christian Couder
When looking for the start of the trailers in the message
we are passed, we should ignore the first line of the message.

The reason is that if we are passed a patch or commit message
then the first line should be the patch title.
If we are passed only trailers we can expect that they start
with an empty line that can be ignored too.

This way we can properly process commit messages that have
only one line with something that looks like a trailer, for
example like area of code: change we made.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t7513-interpret-trailers.sh | 15 ++-
 trailer.c |  3 ++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index bd0ab46..f7ebc5e 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -93,12 +93,25 @@ test_expect_success 'with config option on the command 
line' '
Acked-by: Johan
Reviewed-by: Peff
EOF
-   echo Acked-by: Johan |
+   { echo; echo Acked-by: Johan; } |
git -c trailer.Acked-by.ifexists=addifdifferent interpret-trailers \
--trailer Reviewed-by: Peff --trailer Acked-by: Johan 
actual 
test_cmp expected actual
 '
 
+test_expect_success 'with message that contains only a title' '
+   cat expected -\EOF 
+   area: change
+
+   Reviewed-by: Peff
+   Acked-by: Johan
+   EOF
+   echo area: change |
+   git interpret-trailers --trailer Reviewed-by: Peff \
+   --trailer Acked-by: Johan actual 
+   test_cmp expected actual
+'
+
 test_expect_success 'with config setup' '
git config trailer.ack.key Acked-by:  
cat expected -\EOF 
diff --git a/trailer.c b/trailer.c
index 4b14a56..af80b8e 100644
--- a/trailer.c
+++ b/trailer.c
@@ -740,8 +740,9 @@ static int find_trailer_start(struct strbuf **lines, int 
count)
/*
 * Get the start of the trailers by looking starting from the end
 * for a line with only spaces before lines with one separator.
+* The start cannot be the first line.
 */
-   for (start = count - 1; start = 0; start--) {
+   for (start = count - 1; start = 1; start--) {
if (lines[start]-buf[0] == comment_line_char)
continue;
if (contains_only_spaces(lines[start]-buf)) {
-- 
2.5.0.400.gff86faf.dirty

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