threads / patch / 36641

patch, 2 partsgit-show: fix 'git show -s' to not add extra terminator after merge commit

Subject: [PATCH 1/2] git-show: fix 'git show -s' to not add extra terminator after merge commit

## tl;dr

3 messages between May 12, 2014 and May 12, 2014. Diffs are folded; open one to read it.

replies: 2people: 1as markdown or json

Max Kirillov· May 12, 2014, 22:35 UTC · lore

[PATCH 0/2] fix 'git show -s' to not add extra terminator after merge commit

* fix the CC issues
* add fixes for existing tests
Max Kirillov (2):
  git-show: fix 'git show -s' to not add extra terminator after merge commit
  t: git-show: adapt tests to fixed 'git show'
 combine-diff.c                |  3 ++-
 t/t1507-rev-parse-upstream.sh |  2 +-
 t/t7007-show.sh               |  8 ++++++--
 t/t7600-merge.sh              | 11 +++++------
 4 files changed, 14 insertions(+), 10 deletions(-)
-- 
1.8.5.2.421.g4cdf8d0
Max Kirillov· May 12, 2014, 22:42 UTC · re: Max Kirillov · lore

When git show -s is called for merge commit it prints extra newline after any merge commit and the next one. This looks especially ugly for --oneline and other single-line formats. Looks very much like a bug.

The code in question exists since commit 3969cf7db1. Probably the correct condition should be in fact "opt->output_format & DIFF_FORMAT_DIFFSTAT".

Test t7007-show.sh is also modified to cover this case.
---
 combine-diff.c  | 3 ++-
 t/t7007-show.sh | 8 ++++++--
 2 files changed, 8 insertions(+), 3 deletions(-)
Show changes to 2 files +8 −3

combine-diff.c, t/t7007-show.sh

diff --git a/combine-diff.c b/combine-diff.c
index 3b92c448..ff6ceaf 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1331,7 +1331,8 @@ void diff_tree_combined(const unsigned char *sha1,
 		if (show_log_first && i == 0) {
 			show_log(rev);
 
-			if (rev->verbose_header && opt->output_format)
+			if (rev->verbose_header && opt->output_format &&
+			    opt->output_format != DIFF_FORMAT_NO_OUTPUT)
 				printf("%s%c", diff_line_prefix(opt),
 				       opt->line_termination);
 		}
diff --git a/t/t7007-show.sh b/t/t7007-show.sh
index e41fa00..de22812 100755
--- a/t/t7007-show.sh
+++ b/t/t7007-show.sh
@@ -25,6 +25,7 @@ test_expect_success 'set up a bit of history' '
 	git checkout -b side HEAD^^ &&
 	test_commit side2 &&
 	test_commit side3
+	test_merge merge main3
 '
 
 test_expect_success 'showing two commits' '
@@ -109,8 +110,11 @@ test_expect_success 'showing range' '
 '
 
 test_expect_success '-s suppresses diff' '
-	echo main3 >expect &&
-	git show -s --format=%s main3 >actual &&
+	cat >expect <<-\EOF &&
+	merge
+	main3
+	EOF
+	git show -s --format=%s merge main3 >actual &&
 	test_cmp expect actual
 '
 
-- 
1.8.5.2.421.g4cdf8d0
Max Kirillov· May 12, 2014, 22:43 UTC · re: Max Kirillov · lore

[PATCH 2/2] t: git-show: adapt tests to fixed 'git show'

'git show' used to print extra newline after merge commit, and it was recorded so into the test reference data. Now when the behavior is fixed, the tests should be updated.

Note that '--format=%s' works like '--pretty=tformat:%s'. This
is why non-merging cases pass, like t3505-cherry-pick-empty.sh
for example, though they look very similarly.
---
 t/t1507-rev-parse-upstream.sh |  2 +-
 t/t7600-merge.sh              | 11 +++++------
 2 files changed, 6 insertions(+), 7 deletions(-)
Show changes to 2 files +6 −7

t/t1507-rev-parse-upstream.sh, t/t7600-merge.sh

diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index 2a19e79..672280b 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -100,7 +100,7 @@ test_expect_success 'merge my-side@{u} records the correct name' '
 	git branch -D new ;# can fail but is ok
 	git branch -t new my-side@{u} &&
 	git merge -s ours new@{u} &&
-	git show -s --pretty=format:%s >actual &&
+	git show -s --pretty=tformat:%s >actual &&
 	echo "Merge remote-tracking branch ${sq}origin/side${sq}" >expect &&
 	test_cmp expect actual
 )
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 10aa028..b164621 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -57,11 +57,10 @@ create_merge_msgs () {
 		git log --no-merges ^HEAD c2 c3
 	} >squash.1-5-9 &&
 	: >msg.nologff &&
-	echo >msg.nolognoff &&
+	: >msg.nolognoff &&
 	{
 		echo "* tag 'c3':" &&
-		echo "  commit 3" &&
-		echo
+		echo "  commit 3"
 	} >msg.log
 }
 
@@ -71,7 +70,7 @@ verify_merge () {
 	git diff --exit-code &&
 	if test -n "$3"
 	then
-		git show -s --pretty=format:%s HEAD >msg.act &&
+		git show -s --pretty=tformat:%s HEAD >msg.act &&
 		test_cmp "$3" msg.act
 	fi
 }
@@ -620,10 +619,10 @@ test_expect_success 'merge early part of c2' '
 	git tag c6 &&
 	git branch -f c5-branch c5 &&
 	git merge c5-branch~1 &&
-	git show -s --pretty=format:%s HEAD >actual.branch &&
+	git show -s --pretty=tformat:%s HEAD >actual.branch &&
 	git reset --keep HEAD^ &&
 	git merge c5~1 &&
-	git show -s --pretty=format:%s HEAD >actual.tag &&
+	git show -s --pretty=tformat:%s HEAD >actual.tag &&
 	test_cmp expected.branch actual.branch &&
 	test_cmp expected.tag actual.tag
 '
-- 
1.8.5.2.421.g4cdf8d0

← back to recent threads