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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 28, 2013, 02:34 UTC
Message-ID: <20130128023409.GH8206@elie.Belkin>
In-Reply-To: <1359335515-13818-7-git-send-email-drafnel@gmail.com>

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

[...]

Show 6 quoted lines

> @@ -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.)

[...]

Show 7 quoted lines

> --- 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 <jrnieder@gmail.com>