Re: [PATCH 1/8] Use %B for Split Subject/Body

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

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

> Subject: Re: [PATCH 1/8] Use %B for Split Subject/Body

This needs to say "contrib/subtree" somewhere (applies to all
patches in this series).

> 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.
>
> Author:    Techlive Zheng <techlivezheng@gmail.com>

This needs to be a S-o-b instead; is it a real name, by the way?

> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---
>  contrib/subtree/git-subtree.sh     |    5 +++
>  contrib/subtree/t/t7900-subtree.sh |   73 ++++++++++++++++++++++--------------
>  2 files changed, 49 insertions(+), 29 deletions(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 920c664..f2b6d4a 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -296,7 +296,12 @@ 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}"
> +	# 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%s%n%n%b' "$1" |
> +#	git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B' "$1" |

Is it really replacing %s%n%n%b with %B, or is it still an
experiment that is disabled?

>  	(
>  		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 bc2eeb0..93eeb09 100755
> --- a/contrib/subtree/t/t7900-subtree.sh
> +++ b/contrib/subtree/t/t7900-subtree.sh
> @@ -76,6 +76,10 @@ test_expect_success 'add sub1' '
>          git branch -m master subproj
>  '
>  
> +# Save this hash for testing later.
> +
> +subdir_hash=`git rev-parse HEAD`
> +

We prefer $() over ``; much more readable.

>  # 3
>  test_expect_success 'add sub2' '
>          create sub2 &&
> @@ -155,7 +159,6 @@ test_expect_success 'add main-sub5' '
>          create subdir/main-sub5 &&
>          git commit -m "main-sub5"
>  '
> -
>  # 15
>  test_expect_success 'add main6' '
>          create main6 &&

Why?

> @@ -235,7 +238,19 @@ test_expect_success 'check split with --branch' '
>          check_equal ''"$(git rev-parse splitbr1)"'' "$spl1"

Is quoting screwed up around here (and in many other places in this
patch)?  What are these no-op '' doing?

>  '
>  
> -# 25
> +#25

Why the lossage of a SP?

It may make sense to lose these "# num" that will have to be touched
every time somebody inserts new test pieces in the middle, as a
preparatory step before any of these patches, by the way.  That will
reduce noise in the patches for real changes.