{"thread":{"id":"36659","subject":"[PATCH v3] git-show: fix 'git show -s' to not add extra terminator after merge commit","startedAt":"2014-05-14T22:12:45Z","lastAt":"2014-05-15T16:33:08Z","messageCount":2,"participants":["Max Kirillov","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"241591","messageId":"20140514221245.GA22918@wheezy.local","threadId":"36659","inReplyTo":null,"subject":"[PATCH v3] git-show: fix 'git show -s' to not add extra terminator after merge commit","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2014-05-14T22:12:45Z","receivedAt":"2014-05-14T22:12:45Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"When git show -s is called for merge commit it prints extra newline\nafter any merge commit. This differs from output for commits with one\nparent. Fix it by more thorough checking that diff output is disabled.\n\nThe code in question exists since commit 3969cf7db1. The additional\nnewline is really needed for cases when patch is requested, test\nt4013-diff-various.sh contains cases which can demonstrate behavior when\nthe condition is restricted further.\n\nTests:\n\nAdded merge commit to 'set up a bit of history' case in t7007-show.sh to\ncover the fix.\n\nExisting tests are updated to demonstrate the new behaviour.  Earlier,\nthe tests that used \"git show -s --pretty=format:%s\", even though\n\"--pretty=format:%s\" calls for item separator semantics and does not ask\nfor the terminating newline after the last item, expected the output to\nend with such a newline.  They were relying on the buggy behaviour.  Use\nof \"--format=%s\", which is equivalent to \"--pretty=tformat:%s\" that asks\nfor a terminating newline after each item, is a more realistic way to\nuse the command.\n\nIn the test 'merge log messages' the expected data is changed, because\nit was explicitly listing the extra newline. Also the msg.nologff and\nmsg.nolognoff expected files are replaced by one msg.nolog, because they\nwere diffing because of the bug, and now there should be no difference.\n\nSigned-off-by: Max Kirillov <max@max630.net>\n---\nChanges compared to v2:\n* fixed && chaining\n* squashed commit fixing test with commit fixing behavior.\n  Also simplified t7600 a bit - now different expected data\n  are not needed\n* rephrased the commit message. Have made some experiments with\n  changing the condition, added a note about it in place of\n  assumptions. Also commented the tests change. After some\n  struggling to explain choices with own words copied the\n  Junio's paragraph.\n combine-diff.c                |  3 ++-\n t/t1507-rev-parse-upstream.sh |  2 +-\n t/t7007-show.sh               | 10 +++++++---\n t/t7600-merge.sh              | 11 +++++------\n 4 files changed, 15 insertions(+), 11 deletions(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 3b92c448..ff6ceaf 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -1331,7 +1331,8 @@ void diff_tree_combined(const unsigned char *sha1,\n \t\tif (show_log_first && i == 0) {\n \t\t\tshow_log(rev);\n \n-\t\t\tif (rev->verbose_header && opt->output_format)\n+\t\t\tif (rev->verbose_header && opt->output_format &&\n+\t\t\t    opt->output_format != DIFF_FORMAT_NO_OUTPUT)\n \t\t\t\tprintf(\"%s%c\", diff_line_prefix(opt),\n \t\t\t\t       opt->line_termination);\n \t\t}\ndiff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh\nindex 2a19e79..672280b 100755\n--- a/t/t1507-rev-parse-upstream.sh\n+++ b/t/t1507-rev-parse-upstream.sh\n@@ -100,7 +100,7 @@ test_expect_success 'merge my-side@{u} records the correct name' '\n \tgit branch -D new ;# can fail but is ok\n \tgit branch -t new my-side@{u} &&\n \tgit merge -s ours new@{u} &&\n-\tgit show -s --pretty=format:%s >actual &&\n+\tgit show -s --pretty=tformat:%s >actual &&\n \techo \"Merge remote-tracking branch ${sq}origin/side${sq}\" >expect &&\n \ttest_cmp expect actual\n )\ndiff --git a/t/t7007-show.sh b/t/t7007-show.sh\nindex e41fa00..1b824fe 100755\n--- a/t/t7007-show.sh\n+++ b/t/t7007-show.sh\n@@ -24,7 +24,8 @@ test_expect_success 'set up a bit of history' '\n \tgit tag -m \"annotated tag\" annotated &&\n \tgit checkout -b side HEAD^^ &&\n \ttest_commit side2 &&\n-\ttest_commit side3\n+\ttest_commit side3 &&\n+\ttest_merge merge main3\n '\n \n test_expect_success 'showing two commits' '\n@@ -109,8 +110,11 @@ test_expect_success 'showing range' '\n '\n \n test_expect_success '-s suppresses diff' '\n-\techo main3 >expect &&\n-\tgit show -s --format=%s main3 >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tmerge\n+\tmain3\n+\tEOF\n+\tgit show -s --format=%s merge main3 >actual &&\n \ttest_cmp expect actual\n '\n \ndiff --git a/t/t7600-merge.sh b/t/t7600-merge.sh\nindex 10aa028..b164621 100755\n--- a/t/t7600-merge.sh\n+++ b/t/t7600-merge.sh\n@@ -57,11 +57,10 @@ create_merge_msgs () {\n \t\tgit log --no-merges ^HEAD c2 c3\n \t} >squash.1-5-9 &&\n \t: >msg.nologff &&\n-\techo >msg.nolognoff &&\n+\t: >msg.nolognoff &&\n \t{\n \t\techo \"* tag 'c3':\" &&\n-\t\techo \"  commit 3\" &&\n-\t\techo\n+\t\techo \"  commit 3\"\n \t} >msg.log\n }\n \n@@ -71,7 +70,7 @@ verify_merge () {\n \tgit diff --exit-code &&\n \tif test -n \"$3\"\n \tthen\n-\t\tgit show -s --pretty=format:%s HEAD >msg.act &&\n+\t\tgit show -s --pretty=tformat:%s HEAD >msg.act &&\n \t\ttest_cmp \"$3\" msg.act\n \tfi\n }\n@@ -620,10 +619,10 @@ test_expect_success 'merge early part of c2' '\n \tgit tag c6 &&\n \tgit branch -f c5-branch c5 &&\n \tgit merge c5-branch~1 &&\n-\tgit show -s --pretty=format:%s HEAD >actual.branch &&\n+\tgit show -s --pretty=tformat:%s HEAD >actual.branch &&\n \tgit reset --keep HEAD^ &&\n \tgit merge c5~1 &&\n-\tgit show -s --pretty=format:%s HEAD >actual.tag &&\n+\tgit show -s --pretty=tformat:%s HEAD >actual.tag &&\n \ttest_cmp expected.branch actual.branch &&\n \ttest_cmp expected.tag actual.tag\n '\n-- \n1.8.5.2.421.g4cdf8d0\n"},{"id":"241692","messageId":"xmqqwqdngfy3.fsf@gitster.dls.corp.google.com","threadId":"36659","inReplyTo":"20140514221245.GA22918@wheezy.local","subject":"Re: [PATCH v3] git-show: fix 'git show -s' to not add extra terminator after merge commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-15T16:33:08Z","receivedAt":"2014-05-15T16:33:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, will replace the topic.\n"}]}