{"thread":{"id":"32516","subject":"[PATCH v2 0/5] nd/maint-branch-desc-doc reroll","startedAt":"2013-01-03T14:03:06Z","lastAt":"2013-01-03T16:16:37Z","messageCount":8,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":5},"messages":[{"id":"205924","messageId":"1357221791-7496-1-git-send-email-pclouds@gmail.com","threadId":"32516","inReplyTo":null,"subject":"[PATCH v2 0/5] nd/maint-branch-desc-doc reroll","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-03T14:03:06Z","receivedAt":"2013-01-03T14:03:06Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":" - move the comment in 5/5 into the commented code block\n - resolve branch name for \"HEAD\" in 4/5\n\nNguyễn Thái Ngọc Duy (5):\n config.txt: a few lines about branch..description\n branch: delete branch description if it's empty\n t4014: a few more tests on cover letter using branch description\n format-patch: pick up correct branch name from symbolic ref\n format-patch: pick up branch description when no ref is specified\n\n Documentation/config.txt | 6 ++++++\n builtin/branch.c | 2 +-\n builtin/log.c | 33 ++++++++++++++++++++-------------\n t/t4014-format-patch.sh | 42 ++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 69 insertions(+), 14 deletions(-)\n\n-- \n1.8.0.rc2.23.g1fb49df\n"},{"id":"205925","messageId":"1357221791-7496-2-git-send-email-pclouds@gmail.com","threadId":"32516","inReplyTo":"1357221791-7496-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 1/5] config.txt: a few lines about branch..description","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-03T14:03:07Z","receivedAt":"2013-01-03T14:03:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/config.txt | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex bf8f911..ee64846 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -735,6 +735,12 @@ branch..rebase::\n it unless you understand the implications (see linkgit:git-rebase[1]\n for details).\n \n+branch..description::\n+\tBranch description, can be edited with\n+\t`git branch --edit-description`. Branch description is\n+\tautomatically added in the format-patch cover letter or\n+\trequest-pull summary.\n+\n browser..cmd::\n \tSpecify the command to invoke the specified browser. The\n \tspecified command is evaluated in shell with the URLs passed\n-- \n1.8.0.rc2.23.g1fb49df\n"},{"id":"205926","messageId":"1357221791-7496-3-git-send-email-pclouds@gmail.com","threadId":"32516","inReplyTo":"1357221791-7496-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 2/5] branch: delete branch description if it's empty","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-03T14:03:08Z","receivedAt":"2013-01-03T14:03:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/branch.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 1ec9c02..873f624 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -725,7 +725,7 @@ static int edit_branch_description(const char *branch_name)\n \tstripspace(&buf, 1);\n \n \tstrbuf_addf(&name, \"branch.%s.description\", branch_name);\n-\tstatus = git_config_set(name.buf, buf.buf);\n+\tstatus = git_config_set(name.buf, buf.len ? buf.buf : NULL);\n \tstrbuf_release(&name);\n \tstrbuf_release(&buf);\n \n-- \n1.8.0.rc2.23.g1fb49df\n"},{"id":"205927","messageId":"1357221791-7496-4-git-send-email-pclouds@gmail.com","threadId":"32516","inReplyTo":"1357221791-7496-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 3/5] t4014: a few more tests on cover letter using branch description","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-03T14:03:09Z","receivedAt":"2013-01-03T14:03:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n t/t4014-format-patch.sh | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 16a4ca1..ec9ef9e 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -963,4 +963,25 @@ test_expect_success 'format patch ignores color.ui' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'cover letter using branch description (1)' '\n+\tgit checkout rebuild-1 &&\n+\ttest_config branch.rebuild-1.description hello &&\n+\tgit format-patch --stdout --cover-letter master >actual &&\n+\tgrep hello actual >/dev/null\n+'\n+\n+test_expect_success 'cover letter using branch description (2)' '\n+\tgit checkout rebuild-1 &&\n+\ttest_config branch.rebuild-1.description hello &&\n+\tgit format-patch --stdout --cover-letter rebuild-1~2..rebuild-1 >actual &&\n+\tgrep hello actual >/dev/null\n+'\n+\n+test_expect_success 'cover letter using branch description (3)' '\n+\tgit checkout rebuild-1 &&\n+\ttest_config branch.rebuild-1.description hello &&\n+\tgit format-patch --stdout --cover-letter ^master rebuild-1 >actual &&\n+\tgrep hello actual >/dev/null\n+'\n+\n test_done\n-- \n1.8.0.rc2.23.g1fb49df\n"},{"id":"205928","messageId":"1357221791-7496-5-git-send-email-pclouds@gmail.com","threadId":"32516","inReplyTo":"1357221791-7496-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 4/5] format-patch: pick up correct branch name from symbolic ref","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-03T14:03:10Z","receivedAt":"2013-01-03T14:03:10Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"find_branch_name() assumes to take refs/heads/. But we also\nhave symbolic refs, such as HEAD, that can point to a valid branch in\nrefs/heads and do not follow refs/heads/ syntax. Remove the\nassumption and apply normal ref resolution. After all it would be\nconfusing if rev machinery resolves a ref in one way and\nfind_branch_name() another.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/log.c | 21 +++++++++------------\n t/t4014-format-patch.sh | 14 ++++++++++++++\n 2 files changed, 23 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex e7b7db1..039bf67 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1016,8 +1016,8 @@ static char *find_branch_name(struct rev_info *rev)\n {\n \tint i, positive = -1;\n \tunsigned char branch_sha1[20];\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tconst char *branch;\n+\tconst char *ref;\n+\tchar *full_ref, *branch = NULL;\n \n \tfor (i = 0; i < rev->cmdline.nr; i++) {\n \t\tif (rev->cmdline.rev[i].flags & UNINTERESTING)\n@@ -1029,16 +1029,13 @@ static char *find_branch_name(struct rev_info *rev)\n \t}\n \tif (positive < 0)\n \t\treturn NULL;\n-\tstrbuf_addf(&buf, \"refs/heads/%s\", rev->cmdline.rev[positive].name);\n-\tbranch = resolve_ref_unsafe(buf.buf, branch_sha1, 1, NULL);\n-\tif (!branch ||\n-\t prefixcmp(branch, \"refs/heads/\") ||\n-\t hashcmp(rev->cmdline.rev[positive].item->sha1, branch_sha1))\n-\t\tbranch = NULL;\n-\tstrbuf_release(&buf);\n-\tif (branch)\n-\t\treturn xstrdup(rev->cmdline.rev[positive].name);\n-\treturn NULL;\n+\tref = rev->cmdline.rev[positive].name;\n+\tif (dwim_ref(ref, strlen(ref), branch_sha1, &full_ref) &&\n+\t !prefixcmp(full_ref, \"refs/heads/\") &&\n+\t !hashcmp(rev->cmdline.rev[positive].item->sha1, branch_sha1))\n+\t\tbranch = xstrdup(full_ref + strlen(\"refs/heads/\"));\n+\tfree(full_ref);\n+\treturn branch;\n }\n \n int cmd_format_patch(int argc, const char **argv, const char *prefix)\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex ec9ef9e..903a797 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -984,4 +984,18 @@ test_expect_success 'cover letter using branch description (3)' '\n \tgrep hello actual >/dev/null\n '\n \n+test_expect_success 'cover letter using branch description (4)' '\n+\tgit checkout rebuild-1 &&\n+\ttest_config branch.rebuild-1.description hello &&\n+\tgit format-patch --stdout --cover-letter master.. >actual &&\n+\tgrep hello actual >/dev/null\n+'\n+\n+test_expect_success 'cover letter using branch description (5)' '\n+\tgit checkout rebuild-1 &&\n+\ttest_config branch.rebuild-1.description hello &&\n+\tgit format-patch --stdout --cover-letter -2 HEAD >actual &&\n+\tgrep hello actual >/dev/null\n+'\n+\n test_done\n-- \n1.8.0.rc2.23.g1fb49df\n"},{"id":"205929","messageId":"1357221791-7496-6-git-send-email-pclouds@gmail.com","threadId":"32516","inReplyTo":"1357221791-7496-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 5/5] format-patch: pick up branch description when no ref is specified","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-03T14:03:11Z","receivedAt":"2013-01-03T14:03:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"We only try to get branch name in \"format-patch origin\" case or\nsimilar and not \"format-patch -22\" where HEAD is automatically\nadded. Without correct branch name, branch description cannot be\nadded. Make sure we always get branch name.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/log.c | 16 +++++++++++++---\n t/t4014-format-patch.sh | 7 +++++++\n 2 files changed, 20 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 039bf67..81683f6 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1027,12 +1027,22 @@ static char *find_branch_name(struct rev_info *rev)\n \t\telse\n \t\t\treturn NULL;\n \t}\n-\tif (positive < 0)\n+\tif (positive >= 0)\n+\t\tref = rev->cmdline.rev[positive].name;\n+\telse if (!rev->cmdline.nr && rev->pending.nr == 1 &&\n+\t\t !strcmp(rev->pending.objects[0].name, \"HEAD\"))\n+\t\t/*\n+\t\t * No actual ref from command line, but \"HEAD\" from\n+\t\t * rev->def was added in setup_revisions()\n+\t\t * e.g. format-patch --cover-letter -12\n+\t\t */\n+\t\tref = \"HEAD\";\n+\telse\n \t\treturn NULL;\n-\tref = rev->cmdline.rev[positive].name;\n \tif (dwim_ref(ref, strlen(ref), branch_sha1, &full_ref) &&\n \t !prefixcmp(full_ref, \"refs/heads/\") &&\n-\t !hashcmp(rev->cmdline.rev[positive].item->sha1, branch_sha1))\n+\t (positive < 0 ||\n+\t !hashcmp(rev->cmdline.rev[positive].item->sha1, branch_sha1)))\n \t\tbranch = xstrdup(full_ref + strlen(\"refs/heads/\"));\n \tfree(full_ref);\n \treturn branch;\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 903a797..7e52941 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -998,4 +998,11 @@ test_expect_success 'cover letter using branch description (5)' '\n \tgrep hello actual >/dev/null\n '\n \n+test_expect_success 'cover letter using branch description (6)' '\n+\tgit checkout rebuild-1 &&\n+\ttest_config branch.rebuild-1.description hello &&\n+\tgit format-patch --stdout --cover-letter -2 >actual &&\n+\tgrep hello actual >/dev/null\n+'\n+\n test_done\n-- \n1.8.0.rc2.23.g1fb49df\n"},{"id":"205933","messageId":"7v7gnuxo4q.fsf@alter.siamese.dyndns.org","threadId":"32516","inReplyTo":"1357221791-7496-6-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2 5/5] format-patch: pick up branch description when no ref is specified","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-03T15:49:41Z","receivedAt":"2013-01-03T15:49:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> We only try to get branch name in \"format-patch origin\" case or\n> similar and not \"format-patch -22\" where HEAD is automatically\n> added. Without correct branch name, branch description cannot be\n> added. Make sure we always get branch name.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> builtin/log.c | 16 +++++++++++++---\n> t/t4014-format-patch.sh | 7 +++++++\n> 2 files changed, 20 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 039bf67..81683f6 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1027,12 +1027,22 @@ static char *find_branch_name(struct rev_info *rev)\n> \t\telse\n> \t\t\treturn NULL;\n> \t}\n> -\tif (positive < 0)\n> +\tif (positive >= 0)\n> +\t\tref = rev->cmdline.rev[positive].name;\n> +\telse if (!rev->cmdline.nr && rev->pending.nr == 1 &&\n> +\t\t !strcmp(rev->pending.objects[0].name, \"HEAD\"))\n> +\t\t/*\n> +\t\t * No actual ref from command line, but \"HEAD\" from\n> +\t\t * rev->def was added in setup_revisions()\n> +\t\t * e.g. format-patch --cover-letter -12\n> +\t\t */\n> +\t\tref = \"HEAD\";\n> +\telse\n> \t\treturn NULL;\n> -\tref = rev->cmdline.rev[positive].name;\n> \tif (dwim_ref(ref, strlen(ref), branch_sha1, &full_ref) &&\n> \t !prefixcmp(full_ref, \"refs/heads/\") &&\n> -\t !hashcmp(rev->cmdline.rev[positive].item->sha1, branch_sha1))\n> +\t (positive < 0 ||\n> +\t !hashcmp(rev->cmdline.rev[positive].item->sha1, branch_sha1)))\n\nThis hashcmp() is to make sure that the tip we _guessed_ with\ndwim_ref() that the user meant matches what we are going to use, so\nI do not think disabling the check is a good idea.\n\nYou could (and I think you should) do something like this:\n\n\tif (0 <= positive) {\n \tref = rev->cmdline.rev[positive].name;\n tip_sha1 = rev->cmdline.rev[positive].item->sha1;\n\t} else if (... defaulted to implied HEAD? ...) {\n\t\tref = \"HEAD\";\n tip_sha1 = rev->pending.objects[0].item->sha1;\n\t} else {\n\t\treturn NULL;\n\t}\n\n if (dwim_ref(...) && !prefixcmp(full_ref, \"refs/heads/\") &&\n !hashcmp(tip_sha1, branch_sha1))\n\nto preserve that safety instead.\n"},{"id":"205935","messageId":"1357229797-12260-1-git-send-email-pclouds@gmail.com","threadId":"32516","inReplyTo":"7v7gnuxo4q.fsf@alter.siamese.dyndns.org","subject":"[PATCH v3 5/5] format-patch: pick up branch description when no ref is specified","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-03T16:16:37Z","receivedAt":"2013-01-03T16:16:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"We only try to get branch name in \"format-patch origin\" case or\nsimilar and not \"format-patch -22\" where HEAD is automatically\nadded. Without correct branch name, branch description cannot be\nadded. Make sure we always get branch name.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n On Thu, Jan 3, 2013 at 10:49 PM, Junio C Hamano wrote:\n > You could (and I think you should) do something like this:\n >\n > if (0 <= positive) {\n > ref = rev->cmdline.rev[positive].name;\n > tip_sha1 = rev->cmdline.rev[positive].item->sha1;\n > } else if (... defaulted to implied HEAD? ...) {\n > ref = \"HEAD\";\n > tip_sha1 = rev->pending.objects[0].item->sha1;\n > } else {\n > return NULL;\n > }\n >\n > if (dwim_ref(...) && !prefixcmp(full_ref, \"refs/heads/\") &&\n > !hashcmp(tip_sha1, branch_sha1))\n >\n > to preserve that safety instead.\n\n Good idea. Done.\n\n builtin/log.c | 18 +++++++++++++++---\n t/t4014-format-patch.sh | 7 +++++++\n 2 files changed, 22 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 039bf67..72a368a 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1016,6 +1016,7 @@ static char *find_branch_name(struct rev_info *rev)\n {\n \tint i, positive = -1;\n \tunsigned char branch_sha1[20];\n+\tconst unsigned char *tip_sha1;\n \tconst char *ref;\n \tchar *full_ref, *branch = NULL;\n \n@@ -1027,12 +1028,23 @@ static char *find_branch_name(struct rev_info *rev)\n \t\telse\n \t\t\treturn NULL;\n \t}\n-\tif (positive < 0)\n+\tif (0 <= positive) {\n+\t\tref = rev->cmdline.rev[positive].name;\n+\t\ttip_sha1 = rev->cmdline.rev[positive].item->sha1;\n+\t} else if (!rev->cmdline.nr && rev->pending.nr == 1 &&\n+\t\t !strcmp(rev->pending.objects[0].name, \"HEAD\")) {\n+\t\t/*\n+\t\t * No actual ref from command line, but \"HEAD\" from\n+\t\t * rev->def was added in setup_revisions()\n+\t\t * e.g. format-patch --cover-letter -12\n+\t\t */\n+\t\tref = \"HEAD\";\n+\t\ttip_sha1 = rev->pending.objects[0].item->sha1;\n+\t} else\n \t\treturn NULL;\n-\tref = rev->cmdline.rev[positive].name;\n \tif (dwim_ref(ref, strlen(ref), branch_sha1, &full_ref) &&\n \t !prefixcmp(full_ref, \"refs/heads/\") &&\n-\t !hashcmp(rev->cmdline.rev[positive].item->sha1, branch_sha1))\n+\t !hashcmp(tip_sha1, branch_sha1))\n \t\tbranch = xstrdup(full_ref + strlen(\"refs/heads/\"));\n \tfree(full_ref);\n \treturn branch;\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 903a797..7e52941 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -998,4 +998,11 @@ test_expect_success 'cover letter using branch description (5)' '\n \tgrep hello actual >/dev/null\n '\n \n+test_expect_success 'cover letter using branch description (6)' '\n+\tgit checkout rebuild-1 &&\n+\ttest_config branch.rebuild-1.description hello &&\n+\tgit format-patch --stdout --cover-letter -2 >actual &&\n+\tgrep hello actual >/dev/null\n+'\n+\n test_done\n-- \n1.8.0.rc2.23.g1fb49df\n"}]}