Re: [PATCH v3 06/11] sequencer.c: always separate (cherry picked from from commit body

[Brandon Casey]

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

 --- a/sequencer.c
 +++ b/sequencer.c

 @@ -497,6 +558,8 @@ static int do_pick_commit(struct commit *commit, struct 
 replay_opts *opts)
   }

   if (opts-record_origin) {
 + if (!has_conforming_footer(msgbuf, 0))
 + strbuf_addch(msgbuf, '\n');

 What should this do in the case of an entirely blank message?
 (Maybe that's too insane a case to worry about.)

Yeah, I think there are a number of insane cases that I think we
should just say behavior is undefined.

  - Entirely blank message
  - Message lacking trailing newline.
  - Oneline message that is actually a signed-off-by (equal or not to
the committer's sob).
  - Message body that is entirely 1 or more newlines, or one which has
more than one trailing newline.

It's probably not worth going through contortions to handle these cases.

-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

Re: [PATCH v3 06/11] sequencer.c: always separate (cherry picked from from commit body

[Jonathan Nieder]

Brandon Casey wrote:

 --- a/sequencer.c
 +++ b/sequencer.c
 @@ -20,6 +20,67 @@
[...]
  static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
[...]
 + /* require at least one blank line */
 + if (!last_char_was_nl || buf[i] != '\n')
 + return 0;

Makes sense.  append_signoff already added a blank line after a
one-line message

foo: bar

because of e5138436 (builtin-commit.c: fix logic to omit empty line
before existing footers, 2009-11-06) but the logic to do so was in the
wrong place and it didn't handle its ill-formatted cousin

foo: bar
baz: qux

[...]
 @@ -497,6 +558,8 @@ static int do_pick_commit(struct commit *commit, struct 
 replay_opts *opts)
   }
  
   if (opts-record_origin) {
 + if (!has_conforming_footer(msgbuf, 0))
 + strbuf_addch(msgbuf, '\n');

What should this do in the case of an entirely blank message?
(Maybe that's too insane a case to worry about.)

[...]
 --- a/t/t3511-cherry-pick-x.sh
 +++ b/t/t3511-cherry-pick-x.sh
 @@ -81,6 +94,19 @@ test_expect_failure 'cherry-pick -s inserts blank line 
 after non-conforming foot
   test_cmp expect actual
  '
  
 +test_expect_success 'cherry-pick -x inserts blank line when conforming 
 footer not found' '

Yay. :)

Thanks for a clear and well thought-out patch with thorough tests.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
--
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