Re: [PATCH 2/7] contrib/subtree: Use %B for Split Subject/Body

[Junio C Hamano <gitster@pobox.com>]

"David A. Greene" <greened@obbligato.org> writes:

> From: Techlive Zheng <techlivezheng@gmail.com>
>
> Use %B to format the commit message and body to avoid an extra newline
> if a commit only has a subject line.
>
> Signed-off-by: Techlive Zheng <techlivezheng@gmail.com>
>
> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---

This time (only), I'll try to fix them up at my end, but please
check your toolchain, find out where the extra blank line between
S-o-b: lines we see above come from, and fix that, so that I won't
have to do so again.

>  contrib/subtree/git-subtree.sh     |    6 +++++-
>  contrib/subtree/t/t7900-subtree.sh |   15 +++++++++++++++
>  2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 920c664..5341b36 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -296,7 +296,11 @@ copy_commit()
>  	# We're going to set some environment vars here, so
>  	# do it in a subshell to get rid of them safely later
>  	debug copy_commit "{$1}" "{$2}" "{$3}"
> -	git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%s%n%n%b' "$1" |
> +	# Use %B rather than %s%n%n%b to handle the special case of a
> +	# commit that only has a subject line.  We don't want to
> +	# introduce a newline after the subject, causing generation of
> +	# a new hash.
> +	git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B' "$1" |

The new format template is fine, but I do not think the comment
should be there.  It does not give any useful information to people
who are reading the end result of applying this patch and is useful
only in the context of comparing the old and new templates, iow, it
belongs to the commit log message.

>  	(
>  		read GIT_AUTHOR_NAME
>  		read GIT_AUTHOR_EMAIL
> diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
> index 6cf9fb9..3f17f55 100755
> --- a/contrib/subtree/t/t7900-subtree.sh
> +++ b/contrib/subtree/t/t7900-subtree.sh
> @@ -74,6 +74,10 @@ test_expect_success 'add sub1' '
>          git branch -m master subproj
>  '
>  
> +# Save this hash for testing later.
> +
> +subdir_hash=`git rev-parse HEAD`
> +
>  test_expect_success 'add sub2' '
>          create sub2 &&
>          git commit -m "sub2" &&
> @@ -211,6 +215,17 @@ test_expect_success 'check split with --branch' '
>          check_equal ''"$(git rev-parse splitbr1)"'' "$spl1"
>  '
>  
> +test_expect_success 'check hash of split' '
> +        spl1=$(git subtree split --prefix subdir) &&
> +        undo &&
> +        git subtree split --prefix subdir --branch splitbr1test &&
> +        check_equal ''"$(git rev-parse splitbr1test)"'' "$spl1"

We'd need to clean up these no-op '' from this, but not doing so in
this patch is perfectly fine (and is even preferred).

> +        git checkout splitbr1test &&
> +        new_hash=$(git rev-parse HEAD~2) &&
> +        git checkout mainline &&
> +        check_equal ''"$new_hash"'' "$subdir_hash"
> +'
> +
>  test_expect_success 'check split with --branch for an existing branch' '
>          spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
>          undo &&

Thanks.