{"thread":{"id":"23888","subject":"[PATCH 1/3] t7502-commit: add tests for summary output","startedAt":"2010-05-24T09:47:29Z","lastAt":"2010-06-12T14:15:39Z","messageCount":17,"participants":["Tay Ray Chuan","Junio C Hamano","Will Palmer"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"142192","messageId":"1274694452-4200-1-git-send-email-rctay89@gmail.com","threadId":"23888","inReplyTo":null,"subject":"[PATCH 0/3] commit: fix abbrev-sha regression","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-05-24T09:47:29Z","receivedAt":"2010-05-24T09:47:29Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"After c197702 (pretty: Respect --abbrev option), the summary output for\ngit-commit began to throw up non-abbreviated SHA-1s.\n\nFor example,\n\n $ git commit --allow-empty -m \"empty\"\n [master f869bf09c8eefee54094bb21387241aaf5f10324] empty\n\nI also notice that this happens for merges after conflict resolutions.\n\nThis afflicts 'master', but not any release.\n\nHere's the hunk that fixed it:\n\n-->8--\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex a4e4966..ab4a7cf 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1166,6 +1166,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n \t\tstruct pretty_print_context ctx = {0};\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\tctx.date_mode = DATE_NORMAL;\n+\t\tctx.abbrev = DEFAULT_ABBREV;\n \t\tformat_commit_message(commit, format.buf + 7, &buf, &ctx);\n \t\tprintf(\"%s\\n\", buf.buf);\n \t\tstrbuf_release(&buf);\n-->8--\n\nHowever, I noticed that this would also work:\n\n-->8--\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex ab4a7cf..13a30ab 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1148,7 +1148,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n rev.verbose_header = 1;\n rev.show_root_diff = 1;\n get_commit_format(format.buf, &rev);\n- rev.always_show_header = 0;\n+ rev.always_show_header = 1;\n rev.diffopt.detect_rename = 1;\n rev.diffopt.rename_limit = 100;\n rev.diffopt.break_opt = 0;\n-->8--\n\nThis would make the code block that handled the situation in which\nlog_tree_commit() did not print any output redundant, so perhaps this\nwould also follow:\n\n-->8--\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex ab4a7cf..e8def55 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1163,13 +1163,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n initial_commit ? \" (root-commit)\" : \"\");\n\n if (!log_tree_commit(&rev, commit)) {\n- struct pretty_print_context ctx = {0};\n- struct strbuf buf = STRBUF_INIT;\n- ctx.date_mode = DATE_NORMAL;\n- ctx.abbrev = DEFAULT_ABBREV;\n- format_commit_message(commit, format.buf + 7, &buf, &ctx);\n- printf(\"%s\\n\", buf.buf);\n- strbuf_release(&buf);\n+ die(\"unable to print summary\");\n }\n strbuf_release(&format);\n }\n-->8--\n\nIn a sense, it reverts parts of bf82a15 (commit: do not add extra LF at\nthe end of the summary.), except for the extra LF. Would this be a more\nsensible route?\n\nContents:\n[PATCH 0/3] commit: fix abbrev-sha regression\n[PATCH 1/3] t7502-commit: add tests for summary output\n[PATCH 2/3] t7502-commit: add summary output tests for empty and merge commits\n[PATCH 3/3] commit: show abbreviated sha for commits with empty diffs\n"},{"id":"142189","messageId":"1274694452-4200-2-git-send-email-rctay89@gmail.com","threadId":"23888","inReplyTo":"1274694452-4200-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 1/3] t7502-commit: add tests for summary output","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-05-24T09:47:30Z","receivedAt":"2010-05-24T09:47:30Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Signed-off-by: Tay Ray Chuan \n---\n t/t7502-commit.sh | 36 ++++++++++++++++++++++++++++++++++++\n 1 files changed, 36 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 844fb43..589e8e6 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -4,8 +4,44 @@ test_description='git commit porcelain-ish'\n \n . ./test-lib.sh\n \n+# Arguments: []\n+check_summary_oneline() {\n+\ttest_tick &&\n+\tgit commit -m \"$2\" | head -1 > act &&\n+\n+\t# branch name\n+\tSUMMARY_PREFIX=\"$(git name-rev --name-only HEAD)\" &&\n+\n+\t# append the \"special\" prefix, like \"root-commit\", \"detached HEAD\"\n+\tif test -n \"$1\"\n+\tthen\n+\t\tSUMMARY_PREFIX=\"$SUMMARY_PREFIX ($1)\"\n+\tfi\n+\n+\t# abbrev SHA-1\n+\tSUMMARY_POSTFIX=\"$(git log -1 --pretty='format:%h')\"\n+\techo \"[$SUMMARY_PREFIX $SUMMARY_POSTFIX] $2\" >exp &&\n+\n+\ttest_cmp exp act\n+}\n+\n+test_expect_success 'output summary format' '\n+\n+\techo new >file1 &&\n+\tgit add file1 &&\n+\tcheck_summary_oneline \"root-commit\" \"initial\" &&\n+\n+\techo change >>file1 &&\n+\tgit add file1 &&\n+\tcheck_summary_oneline \"\" \"a change\"\n+'\n+\n test_expect_success 'the basics' '\n \n+\t# this is needed for the \"partial removal\" test to pass\n+\tgit rm file1 &&\n+\tgit commit -m \"cleanup\" &&\n+\n \techo doing partial >\"commit is\" &&\n \tmkdir not &&\n \techo very much encouraged but we should >not/forbid &&\n-- \n1.7.1.189.g07419\n"},{"id":"142191","messageId":"1274694452-4200-3-git-send-email-rctay89@gmail.com","threadId":"23888","inReplyTo":"1274694452-4200-2-git-send-email-rctay89@gmail.com","subject":"[PATCH 2/3] t7502-commit: add summary output tests for empty and merge commits","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-05-24T09:47:31Z","receivedAt":"2010-05-24T09:47:31Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"After c197702 (pretty: Respect --abbrev option), non-abbreviated hashes\nbegan to appear, leading to failures for these tests.\n\nSigned-off-by: Tay Ray Chuan \n---\n t/t7502-commit.sh | 29 +++++++++++++++++++++++++++--\n 1 files changed, 27 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 589e8e6..8a4a277 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -4,10 +4,10 @@ test_description='git commit porcelain-ish'\n \n . ./test-lib.sh\n \n-# Arguments: []\n+# Arguments: [] []\n check_summary_oneline() {\n \ttest_tick &&\n-\tgit commit -m \"$2\" | head -1 > act &&\n+\techo \"$3\" | xargs git commit -m \"$2\" | head -1 > act &&\n \n \t# branch name\n \tSUMMARY_PREFIX=\"$(git name-rev --name-only HEAD)\" &&\n@@ -36,6 +36,31 @@ test_expect_success 'output summary format' '\n \tcheck_summary_oneline \"\" \"a change\"\n '\n \n+test_expect_failure 'output summary format for commit with an empty diff' '\n+\n+\tcheck_summary_oneline \"\" \"empty\" \"--allow-empty\"\n+'\n+\n+test_expect_failure 'output summary format for merges' '\n+\n+\tgit checkout -b recursive-base &&\n+\ttest_commit base file1 &&\n+\n+\tgit checkout -b recursive-a recursive-base &&\n+\ttest_commit commit-a file1 &&\n+\n+\tgit checkout -b recursive-b recursive-base &&\n+\ttest_commit commit-b file1 &&\n+\n+\t# conflict\n+\tgit checkout recursive-a &&\n+\ttest_must_fail git merge recursive-b &&\n+\t# resolve the conflict\n+\techo commit-a > file1 &&\n+\tgit add file1 &&\n+\tcheck_summary_oneline \"\" \"Merge\"\n+'\n+\n test_expect_success 'the basics' '\n \n \t# this is needed for the \"partial removal\" test to pass\n-- \n1.7.1.189.g07419\n"},{"id":"142190","messageId":"1274694452-4200-4-git-send-email-rctay89@gmail.com","threadId":"23888","inReplyTo":"1274694452-4200-3-git-send-email-rctay89@gmail.com","subject":"[PATCH 3/3] commit: show abbreviated sha for commits with empty diffs","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-05-24T09:47:32Z","receivedAt":"2010-05-24T09:47:32Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Signed-off-by: Tay Ray Chuan \n---\n builtin/commit.c | 1 +\n t/t7502-commit.sh | 4 ++--\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex a4e4966..ab4a7cf 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1166,6 +1166,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n \t\tstruct pretty_print_context ctx = {0};\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\tctx.date_mode = DATE_NORMAL;\n+\t\tctx.abbrev = DEFAULT_ABBREV;\n \t\tformat_commit_message(commit, format.buf + 7, &buf, &ctx);\n \t\tprintf(\"%s\\n\", buf.buf);\n \t\tstrbuf_release(&buf);\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 8a4a277..109ae00 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -36,12 +36,12 @@ test_expect_success 'output summary format' '\n \tcheck_summary_oneline \"\" \"a change\"\n '\n \n-test_expect_failure 'output summary format for commit with an empty diff' '\n+test_expect_success 'output summary format for commit with an empty diff' '\n \n \tcheck_summary_oneline \"\" \"empty\" \"--allow-empty\"\n '\n \n-test_expect_failure 'output summary format for merges' '\n+test_expect_success 'output summary format for merges' '\n \n \tgit checkout -b recursive-base &&\n \ttest_commit base file1 &&\n-- \n1.7.1.189.g07419\n"},{"id":"142309","messageId":"7viq6bqmid.fsf@alter.siamese.dyndns.org","threadId":"23888","inReplyTo":"1274694452-4200-4-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 3/3] commit: show abbreviated sha for commits with empty diffs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-05-26T05:07:22Z","receivedAt":"2010-05-26T05:07:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The fix itself might be trivial, but the series seems to break 7502.20\nand 7502.22\n\nThanks.\n"},{"id":"142312","messageId":"7vvdabp7xa.fsf@alter.siamese.dyndns.org","threadId":"23888","inReplyTo":"1274694452-4200-3-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 2/3] t7502-commit: add summary output tests for empty and merge commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-05-26T05:07:45Z","receivedAt":"2010-05-26T05:07:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tay Ray Chuan writes:\n\n> After c197702 (pretty: Respect --abbrev option), non-abbreviated hashes\n> began to appear, leading to failures for these tests.\n>\n> Signed-off-by: Tay Ray Chuan \n> ---\n> t/t7502-commit.sh | 29 +++++++++++++++++++++++++++--\n> 1 files changed, 27 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\n> index 589e8e6..8a4a277 100755\n> --- a/t/t7502-commit.sh\n> +++ b/t/t7502-commit.sh\n> @@ -4,10 +4,10 @@ test_description='git commit porcelain-ish'\n> \n> . ./test-lib.sh\n> \n> -# Arguments: []\n> +# Arguments: [] []\n> check_summary_oneline() {\n> \ttest_tick &&\n> -\tgit commit -m \"$2\" | head -1 > act &&\n> +\techo \"$3\" | xargs git commit -m \"$2\" | head -1 > act &&\n\nWhy do you have to fork xargs? Wouldn't/shouldn't\n\n git commit ${3+\"$3\"} -m \"$2\"\n\nwork?\n"},{"id":"142313","messageId":"AANLkTikuukaZNVY8ikn3v1zdzGHUDLBC55fGih7Dgs7L@mail.gmail.com","threadId":"23888","inReplyTo":"7vvdabp7xa.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] t7502-commit: add summary output tests for empty and merge commits","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-05-26T05:19:42Z","receivedAt":"2010-05-26T05:19:42Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Wed, May 26, 2010 at 1:07 PM, Junio C Hamano wrote:\n> Tay Ray Chuan writes:\n>\n>> After c197702 (pretty: Respect --abbrev option), non-abbreviated hashes\n>> began to appear, leading to failures for these tests.\n>>\n>> Signed-off-by: Tay Ray Chuan \n>> ---\n>>  t/t7502-commit.sh |   29 +++++++++++++++++++++++++++--\n>>  1 files changed, 27 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\n>> index 589e8e6..8a4a277 100755\n>> --- a/t/t7502-commit.sh\n>> +++ b/t/t7502-commit.sh\n>> @@ -4,10 +4,10 @@ test_description='git commit porcelain-ish'\n>>\n>>  . ./test-lib.sh\n>>\n>> -# Arguments: []\n>> +# Arguments: [] []\n>>  check_summary_oneline() {\n>>       test_tick &&\n>> -     git commit -m \"$2\" | head -1 > act &&\n>> +     echo \"$3\" | xargs git commit -m \"$2\" | head -1 > act &&\n>\n> Why do you have to fork xargs?  Wouldn't/shouldn't\n>\n>    git commit ${3+\"$3\"} -m \"$2\"\n>\n> work?\n\nIt would. Blame it on my poor shell-fu. :)\n\n-- \nCheers,\nRay Chuan\n"},{"id":"142314","messageId":"AANLkTik_R4U3SvUwLR1nFZhszhmVC8SCmp0JhjGFbUsO@mail.gmail.com","threadId":"23888","inReplyTo":"7viq6bqmid.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] commit: show abbreviated sha for commits with empty diffs","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-05-26T05:37:33Z","receivedAt":"2010-05-26T05:37:33Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Wed, May 26, 2010 at 1:07 PM, Junio C Hamano wrote:\n> The fix itself might be trivial, but the series seems to break 7502.20\n> and 7502.22\n\nI tested this patch series on top of:\n\n - efd1311 (Merge branch 'jn/shortlog' into next), which contains\n c197702 (pretty: Respect --abbrev option), the first offending commit\n to print un-abbreviated SHA-1s.\n\n - b26ba11, the recent 'next'.\n\nIt both cases, #20 and #22 passed fine.\n\nAlso, I outlined another possible fix in the cover letter to this\nseries:\n\n 1274694452-4200-1-git-send-email-rctay89@gmail.com\n http://thread.gmane.org/gmane.comp.version-control.git/147619\n\nI find the alternative fix a bit more precise; I'd appreciate your\ncomments, if you get the chance.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"142315","messageId":"AANLkTinio71lh1y65IW7FKuVFI3PKPPyGc8LNTOT6yqt@mail.gmail.com","threadId":"23888","inReplyTo":"AANLkTik_R4U3SvUwLR1nFZhszhmVC8SCmp0JhjGFbUsO@mail.gmail.com","subject":"Re: [PATCH 3/3] commit: show abbreviated sha for commits with empty diffs","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-05-26T05:39:20Z","receivedAt":"2010-05-26T05:39:20Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Wed, May 26, 2010 at 1:37 PM, Tay Ray Chuan wrote:\n> Hi,\n>\n> On Wed, May 26, 2010 at 1:07 PM, Junio C Hamano wrote:\n>> The fix itself might be trivial, but the series seems to break 7502.20\n>> and 7502.22\n>\n> I tested this patch series on top of:\n>\n>  - efd1311 (Merge branch 'jn/shortlog' into next), which contains\n>   c197702 (pretty: Respect --abbrev option), the first offending commit\n>   to print un-abbreviated SHA-1s.\n>\n>  - b26ba11, the recent 'next'.\n>\n> It both cases, #20 and #22 passed fine.\n\nMy apologies - I've already squashed in a fix to address that locally\nand I haven't sent that out yet.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"142445","messageId":"1274974492-4692-1-git-send-email-rctay89@gmail.com","threadId":"23888","inReplyTo":"1274694452-4200-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 0/3] commit: fix abbrev-sha regression","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-05-27T15:34:49Z","receivedAt":"2010-05-27T15:34:49Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"After c197702 (pretty: Respect --abbrev option), the summary output for\ngit-commit began to throw up non-abbreviated SHA-1s.\n\nFor example,\n\n $ git commit --allow-empty -m \"empty\"\n [master f869bf09c8eefee54094bb21387241aaf5f10324] empty\n\nI also notice that this happens for merges after conflict resolutions.\n\nThis afflicts 'master', but not any release.\n\nChanges from v1:\n\n - move cleanup commands into separate function in patch #1.\n - add a cleanup command to patch #1 to accomodate tests #20 and #22.\n - changed shell syntax in patch #2 (based on Junio's suggestion).\n - used a more \"aggressive\" fix in patch #3.\n\nContents:\n[PATCH v2 1/3] t7502-commit: add tests for summary output\n[PATCH v2 2/3] t7502-commit: add summary output tests for empty and merge commits\n[PATCH v2 3/3] commit::print_summary(): set rev_info.always_show_header to 1\n"},{"id":"142447","messageId":"1274974492-4692-2-git-send-email-rctay89@gmail.com","threadId":"23888","inReplyTo":"1274974492-4692-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 1/3] t7502-commit: add tests for summary output","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-05-27T15:34:50Z","receivedAt":"2010-05-27T15:34:50Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Signed-off-by: Tay Ray Chuan \n---\n\nChanges from v1:\n - move cleanup commands into a separate function,\n output_tests_cleanup().\n - add a cleanup command to accomodate tests #20 and #22.\n\n t/t7502-commit.sh | 43 +++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 43 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 844fb43..478b637 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -4,8 +4,51 @@ test_description='git commit porcelain-ish'\n\n . ./test-lib.sh\n\n+# Arguments: []\n+check_summary_oneline() {\n+\ttest_tick &&\n+\tgit commit -m \"$2\" | head -1 > act &&\n+\n+\t# branch name\n+\tSUMMARY_PREFIX=\"$(git name-rev --name-only HEAD)\" &&\n+\n+\t# append the \"special\" prefix, like \"root-commit\", \"detached HEAD\"\n+\tif test -n \"$1\"\n+\tthen\n+\t\tSUMMARY_PREFIX=\"$SUMMARY_PREFIX ($1)\"\n+\tfi\n+\n+\t# abbrev SHA-1\n+\tSUMMARY_POSTFIX=\"$(git log -1 --pretty='format:%h')\"\n+\techo \"[$SUMMARY_PREFIX $SUMMARY_POSTFIX] $2\" >exp &&\n+\n+\ttest_cmp exp act\n+}\n+\n+test_expect_success 'output summary format' '\n+\n+\techo new >file1 &&\n+\tgit add file1 &&\n+\tcheck_summary_oneline \"root-commit\" \"initial\" &&\n+\n+\techo change >>file1 &&\n+\tgit add file1 &&\n+\tcheck_summary_oneline \"\" \"a change\"\n+'\n+\n+output_tests_cleanup() {\n+\t# this is needed for \"do not fire editor in the presence of conflicts\"\n+\tgit checkout master &&\n+\n+\t# this is needed for the \"partial removal\" test to pass\n+\tgit rm file1 &&\n+\tgit commit -m \"cleanup\"\n+}\n+\n test_expect_success 'the basics' '\n\n+\toutput_tests_cleanup &&\n+\n \techo doing partial >\"commit is\" &&\n \tmkdir not &&\n \techo very much encouraged but we should >not/forbid &&\n--\n1.7.1.189.g07419\n"},{"id":"142451","messageId":"1274974492-4692-3-git-send-email-rctay89@gmail.com","threadId":"23888","inReplyTo":"1274974492-4692-2-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 2/3] t7502-commit: add summary output tests for empty and merge commits","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-05-27T15:34:51Z","receivedAt":"2010-05-27T15:34:51Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"After c197702 (pretty: Respect --abbrev option), non-abbreviated hashes\nbegan to appear, leading to failures for these tests.\n\nSigned-off-by: Tay Ray Chuan \n---\n\nChanges from v1:\n - changed shell syntax in check_summary_oneline(), based on Junio's\n suggestion.\n\n t/t7502-commit.sh | 29 +++++++++++++++++++++++++++--\n 1 files changed, 27 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 478b637..b10541d 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -4,10 +4,10 @@ test_description='git commit porcelain-ish'\n\n . ./test-lib.sh\n\n-# Arguments: []\n+# Arguments: [] []\n check_summary_oneline() {\n \ttest_tick &&\n-\tgit commit -m \"$2\" | head -1 > act &&\n+\tgit commit ${3+\"$3\"} -m \"$2\" | head -1 > act &&\n\n \t# branch name\n \tSUMMARY_PREFIX=\"$(git name-rev --name-only HEAD)\" &&\n@@ -36,6 +36,31 @@ test_expect_success 'output summary format' '\n \tcheck_summary_oneline \"\" \"a change\"\n '\n\n+test_expect_failure 'output summary format for commit with an empty diff' '\n+\n+\tcheck_summary_oneline \"\" \"empty\" \"--allow-empty\"\n+'\n+\n+test_expect_failure 'output summary format for merges' '\n+\n+\tgit checkout -b recursive-base &&\n+\ttest_commit base file1 &&\n+\n+\tgit checkout -b recursive-a recursive-base &&\n+\ttest_commit commit-a file1 &&\n+\n+\tgit checkout -b recursive-b recursive-base &&\n+\ttest_commit commit-b file1 &&\n+\n+\t# conflict\n+\tgit checkout recursive-a &&\n+\ttest_must_fail git merge recursive-b &&\n+\t# resolve the conflict\n+\techo commit-a > file1 &&\n+\tgit add file1 &&\n+\tcheck_summary_oneline \"\" \"Merge\"\n+'\n+\n output_tests_cleanup() {\n \t# this is needed for \"do not fire editor in the presence of conflicts\"\n \tgit checkout master &&\n--\n1.7.1.189.g07419\n"},{"id":"142446","messageId":"1274974492-4692-4-git-send-email-rctay89@gmail.com","threadId":"23888","inReplyTo":"1274974492-4692-3-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 3/3] commit::print_summary(): set rev_info.always_show_header to 1","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-05-27T15:34:52Z","receivedAt":"2010-05-27T15:34:52Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"This attempts to fix a regression in git-commit, where non-abbreviated\nSHA-1s were printed in the summary.\n\nOne possible fix would be to set ctx.abbrev to DEFAULT_ABBREV in the\n`if` block. However, we remove this codeblock altogether, and set\nrev.always_show_header.\n\nThis way, we use back the same show_log() mechanism (instead of\nformat_commit_message()).\n\nQuoting log-tree.c:560:\n\n\tshown = log_tree_diff(opt, commit, &log);\n\tif (!shown && opt->loginfo && opt->always_show_header) {\n\t\tlog.parent = NULL;\n\t\tshow_log(opt);\n\t\tshown = 1;\n\t}\n\nThis is the only area that always_show_header is checked, so the\nsetting of this flag should only affect this area.\n\nSigned-off-by: Tay Ray Chuan \n---\n\nChanged from v1:\n - used an alternative fix - see patch message for more.\n\n builtin/commit.c | 13 ++++---------\n t/t7502-commit.sh | 4 ++--\n 2 files changed, 6 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex a4e4966..2884d0c 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1148,7 +1148,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n \trev.verbose_header = 1;\n \trev.show_root_diff = 1;\n \tget_commit_format(format.buf, &rev);\n-\trev.always_show_header = 0;\n+\trev.always_show_header = 1;\n \trev.diffopt.detect_rename = 1;\n \trev.diffopt.rename_limit = 100;\n \trev.diffopt.break_opt = 0;\n@@ -1162,14 +1162,9 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n \t\t\t\thead,\n \t\tinitial_commit ? \" (root-commit)\" : \"\");\n\n-\tif (!log_tree_commit(&rev, commit)) {\n-\t\tstruct pretty_print_context ctx = {0};\n-\t\tstruct strbuf buf = STRBUF_INIT;\n-\t\tctx.date_mode = DATE_NORMAL;\n-\t\tformat_commit_message(commit, format.buf + 7, &buf, &ctx);\n-\t\tprintf(\"%s\\n\", buf.buf);\n-\t\tstrbuf_release(&buf);\n-\t}\n+\tif (!log_tree_commit(&rev, commit))\n+\t\tdie(\"unable to print summary\");\n+\n \tstrbuf_release(&format);\n }\n\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex b10541d..08c0247 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -36,12 +36,12 @@ test_expect_success 'output summary format' '\n \tcheck_summary_oneline \"\" \"a change\"\n '\n\n-test_expect_failure 'output summary format for commit with an empty diff' '\n+test_expect_success 'output summary format for commit with an empty diff' '\n\n \tcheck_summary_oneline \"\" \"empty\" \"--allow-empty\"\n '\n\n-test_expect_failure 'output summary format for merges' '\n+test_expect_success 'output summary format for merges' '\n\n \tgit checkout -b recursive-base &&\n \ttest_commit base file1 &&\n--\n1.7.1.189.g07419\n"},{"id":"142456","messageId":"1274979484.4448.1.camel@walleee","threadId":"23888","inReplyTo":"1274974492-4692-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH v2 0/3] commit: fix abbrev-sha regression","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2010-05-27T16:58:04Z","receivedAt":"2010-05-27T16:58:04Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"On Thu, 2010-05-27 at 23:34 +0800, Tay Ray Chuan wrote:\n> After c197702 (pretty: Respect --abbrev option), the summary output for\n> git-commit began to throw up non-abbreviated SHA-1s.\n\nPlease CC: me on any future versions of this series, if there are any\n-- \n-- Will\n"},{"id":"142513","messageId":"7vd3wf5x7n.fsf@alter.siamese.dyndns.org","threadId":"23888","inReplyTo":"1274974492-4692-4-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH v2 3/3] commit::print_summary(): set rev_info.always_show_header to 1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-05-29T01:10:52Z","receivedAt":"2010-05-29T01:10:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tay Ray Chuan writes:\n\n> This attempts to fix a regression in git-commit, where non-abbreviated\n> SHA-1s were printed in the summary.\n>\n> One possible fix would be to set ctx.abbrev to DEFAULT_ABBREV in the\n> `if` block. However, we remove this codeblock altogether, and set\n> rev.always_show_header.\n>\n> This way, we use back the same show_log() mechanism (instead of\n> format_commit_message()).\n\nI like the removal of the handcrafted call to f-c-m. Thanks.\n\n> Quoting log-tree.c:560:\n>\n> \tshown = log_tree_diff(opt, commit, &log);\n> \tif (!shown && opt->loginfo && opt->always_show_header) {\n> \t\tlog.parent = NULL;\n> \t\tshow_log(opt);\n> \t\tshown = 1;\n> \t}\n>\n> This is the only area that always_show_header is checked, so the\n> setting of this flag should only affect this area.\n\nHmm, but also setting this flag would affect anything that changes\nbehaviour depending on the value of log.parent, no?\n\n> +\tif (!log_tree_commit(&rev, commit))\n> +\t\tdie(\"unable to print summary\");\n\nWhen always_show_header is set, what are the situations where\nlog_tree_commit() might return false? I think your fix depends on the\nfact that it will never return false (which I think is a correct thing to\nassume---after all that is what \"always_show\" means ;-).\n"},{"id":"142516","messageId":"AANLkTim3hT-kloICHNeuodeisYUqccLhEDq_5rCeBRSC@mail.gmail.com","threadId":"23888","inReplyTo":"7vd3wf5x7n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 3/3] commit::print_summary(): set rev_info.always_show_header to 1","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-05-29T01:41:49Z","receivedAt":"2010-05-29T01:41:49Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"On Sat, May 29, 2010 at 9:10 AM, Junio C Hamano wrote:\n> Tay Ray Chuan writes:\n>> Quoting log-tree.c:560:\n>>\n>>       shown = log_tree_diff(opt, commit, &log);\n>>       if (!shown && opt->loginfo && opt->always_show_header) {\n>>               log.parent = NULL;\n>>               show_log(opt);\n>>               shown = 1;\n>>       }\n>>\n>> This is the only area that always_show_header is checked, so the\n>> setting of this flag should only affect this area.\n>\n> Hmm, but also setting this flag would affect anything that changes\n> behaviour depending on the value of log.parent, no?\n\nA cursory look at log-tree.c leads me to believe only show_log() cares\nabout log.parent.\n\nIn any case, if you look a few more lines up, you would notice another\nlog.parent = NULL:\n\n@552\n\tstruct log_info log;\n\tint shown;\n\n\tlog.commit = commit;\n\tlog.parent = NULL;\n\topt->loginfo = &log;\n\n\tshown = log_tree_diff(opt, commit, &log);\n\tif (!shown && opt->loginfo && opt->always_show_header) {\n\t\tlog.parent = NULL;\n\t\tshow_log(opt);\n\t\tshown = 1;\n\t}\n\nTherefore, I suspect that log.parent = NULL is a kind of\ninitialization for show_log() (log_tree_diff() does call show_log()\ntoo).\n\n>> +     if (!log_tree_commit(&rev, commit))\n>> +             die(\"unable to print summary\");\n>\n> When always_show_header is set, what are the situations where\n> log_tree_commit() might return false?  I think your fix depends on the\n> fact that it will never return false (which I think is a correct thing to\n> assume---after all that is what \"always_show\" means ;-).\n\nBased on my reading, I can't think of any.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"143568","messageId":"1276352139-2272-1-git-send-email-rctay89@gmail.com","threadId":"23888","inReplyTo":"1274974492-4692-4-git-send-email-rctay89@gmail.com","subject":"[PATCH v6 3/3] commit::print_summary(): don't use format_commit_message()","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-06-12T14:15:39Z","receivedAt":"2010-06-12T14:15:39Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"This attempts to fix a regression in git-commit, where non-abbreviated\nSHA-1s were printed in the summary.\n\nOne possible fix would be to set ctx.abbrev to DEFAULT_ABBREV in the\n`if` block, where format_commit_message() is used.\n\nInstead, we do away with the format_commit_message() codeblock\naltogether, replacing it with a re-run of log_tree_commit().\n\nWe re-run log_tree_commit() with rev.always_show_header set, to force\nthe invocation of show_log(). The effect of this flag can be seen from\nthis excerpt from log-tree.c:560, the only area that\nrev.always_show_header is checked:\n\n\tshown = log_tree_diff(opt, commit, &log);\n\tif (!shown && opt->loginfo && opt->always_show_header) {\n\t\tlog.parent = NULL;\n\t\tshow_log(opt);\n\t\tshown = 1;\n\t}\n\nWe also set rev.use_terminator, so that a newline is appended at the end\nof the log message. Note that callers in builtin/log.c that also set\nrev.always_show_header don't have to set rev.use_terminator, but still\nget a newline, because they are wrapped in a pager.\n\nSigned-off-by: Tay Ray Chuan \n---\n\nThis is a reworked version of the third patch of the\n'tc/commit-abbrev-fix' series; there are no changes to the first and\nsecond patches.\n\nChanges from v5:\n\n - don't set rev.always_show_header immediately, so that when there is\n no diff, log_tree_commit() returns false, as before the patch;\n\n - when log_tree_commit() returns false (ie. no diff), set\n rev.always_show_header, and re-run log_tree_commit();\n\n - add a missing newline to summary output by setting\n rev.use_terminator.\n\nSee the patch message for more details.\n\nIn case you're wondering where's v5, it was sent in a private review:\n\n http://github.com/gitster/git/commit/c69160d\n\n builtin/commit.c | 10 ++++------\n t/t7502-commit.sh | 4 ++--\n 2 files changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex a4e4966..aa92362 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1163,13 +1163,11 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n \t\tinitial_commit ? \" (root-commit)\" : \"\");\n\n \tif (!log_tree_commit(&rev, commit)) {\n-\t\tstruct pretty_print_context ctx = {0};\n-\t\tstruct strbuf buf = STRBUF_INIT;\n-\t\tctx.date_mode = DATE_NORMAL;\n-\t\tformat_commit_message(commit, format.buf + 7, &buf, &ctx);\n-\t\tprintf(\"%s\\n\", buf.buf);\n-\t\tstrbuf_release(&buf);\n+\t\trev.always_show_header = 1;\n+\t\trev.use_terminator = 1;\n+\t\tlog_tree_commit(&rev, commit);\n \t}\n+\n \tstrbuf_release(&format);\n }\n\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex b10541d..08c0247 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -36,12 +36,12 @@ test_expect_success 'output summary format' '\n \tcheck_summary_oneline \"\" \"a change\"\n '\n\n-test_expect_failure 'output summary format for commit with an empty diff' '\n+test_expect_success 'output summary format for commit with an empty diff' '\n\n \tcheck_summary_oneline \"\" \"empty\" \"--allow-empty\"\n '\n\n-test_expect_failure 'output summary format for merges' '\n+test_expect_success 'output summary format for merges' '\n\n \tgit checkout -b recursive-base &&\n \ttest_commit base file1 &&\n--\n1.7.1.189.g07419\n"}]}