{"thread":{"id":"32494","subject":"[PATCH 1/3] config.txt: a few lines about branch..description","startedAt":"2013-01-01T09:30:53Z","lastAt":"2013-01-02T01:25:55Z","messageCount":5,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"205760","messageId":"1357032655-21103-1-git-send-email-pclouds@gmail.com","threadId":"32494","inReplyTo":null,"subject":"[PATCH 1/3] config.txt: a few lines about branch..description","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-01T09:30:53Z","receivedAt":"2013-01-01T09:30:53Z","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":"205762","messageId":"1357032655-21103-2-git-send-email-pclouds@gmail.com","threadId":"32494","inReplyTo":"1357032655-21103-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/3] 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-01T09:30:54Z","receivedAt":"2013-01-01T09:30:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"find_branch_name() fails to detect \"format-patch --cover-letter -3\"\nwhere no command line arguments are given and HEAD is automatically\nadded. Detect branch name in this case so we can pick up the branch's\ndescription in cover letter.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/log.c | 16 +++++++++++++++-\n 1 file changed, 15 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex e7b7db1..115f118 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1027,8 +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\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\tif (!rev->cmdline.nr &&\n+\t\t rev->pending.nr == 1 &&\n+\t\t !strcmp(rev->pending.objects[0].name, \"HEAD\")) {\n+\t\t\tconst char *ref;\n+\t\t\tref = resolve_ref_unsafe(\"HEAD\", branch_sha1, 1, NULL);\n+\t\t\tif (ref && !prefixcmp(ref, \"refs/heads/\"))\n+\t\t\t\treturn xstrdup(ref + strlen(\"refs/heads/\"));\n+\t\t}\n \t\treturn NULL;\n+\t}\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-- \n1.8.0.rc2.23.g1fb49df\n"},{"id":"205761","messageId":"1357032655-21103-3-git-send-email-pclouds@gmail.com","threadId":"32494","inReplyTo":"1357032655-21103-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 3/3] branch: delete branch description if it's empty","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-01T09:30:55Z","receivedAt":"2013-01-01T09:30:55Z","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 I don't see the value of having an empty branch description. But I\n may be missing something.\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":"205778","messageId":"7v38ykbpv3.fsf@alter.siamese.dyndns.org","threadId":"32494","inReplyTo":"1357032655-21103-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 2/3] format-patch: pick up branch description when no ref is specified","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-01T20:38:40Z","receivedAt":"2013-01-01T20:38:40Z","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> find_branch_name() fails to detect \"format-patch --cover-letter -3\"\n> where no command line arguments are given and HEAD is automatically\n> added.\n\nNicely spotted.\n\nThat is not the only case that takes this codepath, though.\n\n $ git format-patch --cover-letter master..\n\nwill also give you the same (if you say it without \"..\", which is\nthe more normal invocation of the command, then the caller already\nknow you meant the current branch and this function is not called).\n\nAnd in that case you will have two tokens on cmdline.nr, one for\n\"master..\" to show where he bottom is, and the other for the\nimplied \"HEAD\"; I do not think this patch is a sufficient solution\nfor the more general cases, but on the other hand I do not know how\nmuch it matters.\n\n> -\tif (positive < 0)\n> +\tif (positive < 0) {\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\nThat comment does not describe \"positive < 0\" case, but belongs to\nthe conditional added in this patch, no?\n\n> +\t\tif (!rev->cmdline.nr &&\n> +\t\t rev->pending.nr == 1 &&\n> +\t\t !strcmp(rev->pending.objects[0].name, \"HEAD\")) {\n> +\t\t\tconst char *ref;\n> +\t\t\tref = resolve_ref_unsafe(\"HEAD\", branch_sha1, 1, NULL);\n> +\t\t\tif (ref && !prefixcmp(ref, \"refs/heads/\"))\n> +\t\t\t\treturn xstrdup(ref + strlen(\"refs/heads/\"));\n> +\t\t}\n> \t\treturn NULL;\n> +\t}\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"},{"id":"205823","messageId":"CACsJy8C_rpJrfZLESB8dva4RXrVgcf_Fsnv3zkTw=rhN0JOGog@mail.gmail.com","threadId":"32494","inReplyTo":"7v38ykbpv3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] format-patch: pick up branch description when no ref is specified","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-02T01:25:55Z","receivedAt":"2013-01-02T01:25:55Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Jan 2, 2013 at 3:38 AM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> find_branch_name() fails to detect \"format-patch --cover-letter -3\"\n>> where no command line arguments are given and HEAD is automatically\n>> added.\n>\n> Nicely spotted.\n>\n> That is not the only case that takes this codepath, though.\n>\n> $ git format-patch --cover-letter master..\n>\n> will also give you the same (if you say it without \"..\", which is\n> the more normal invocation of the command, then the caller already\n> know you meant the current branch and this function is not called).\n>\n> And in that case you will have two tokens on cmdline.nr, one for\n> \"master..\" to show where he bottom is, and the other for the\n> implied \"HEAD\";\n\nInteresting. find_brach_name() handles this case wrong because\nrev->cmdline[positive].name is \"HEAD\" and it goes ahead prepending\n\"refs/heads/\" anyway. I'll fix it in the reroll. I was avoiding tests\nwith an excuse that you did not write tests when you added this\nfunction either. But with this change, I think tests are required.\n\n> I do not think this patch is a sufficient solution\n> for the more general cases, but on the other hand I do not know how\n> much it matters.\n\nI think the best place to handle this is setup_revisions() for general\ncases. But we do not need branch detection anywhere else yet, I guess\nit's ok to fix it case by case here. We can move the code back to\nrevisions.c when there are more use cases for it.\n\n>> - if (positive < 0)\n>> + if (positive < 0) {\n>> + /*\n>> + * No actual ref from command line, but \"HEAD\" from\n>> + * rev->def was added in setup_revisions()\n>> + * e.g. format-patch --cover-letter -12\n>> + */\n>\n> That comment does not describe \"positive < 0\" case, but belongs to\n> the conditional added in this patch, no?\n\nIt's meant as the comment for the following block, yes. I'll move it\ninto the block for clarity.\n\n>> + if (!rev->cmdline.nr &&\n>> + rev->pending.nr == 1 &&\n>> + !strcmp(rev->pending.objects[0].name, \"HEAD\")) {\n>> + const char *ref;\n>> + ref = resolve_ref_unsafe(\"HEAD\", branch_sha1, 1, NULL);\n>> + if (ref && !prefixcmp(ref, \"refs/heads/\"))\n>> + return xstrdup(ref + strlen(\"refs/heads/\"));\n>> + }\n>> return NULL;\n>> + }\n>> strbuf_addf(&buf, \"refs/heads/%s\", rev->cmdline.rev[positive].name);\n>> branch = resolve_ref_unsafe(buf.buf, branch_sha1, 1, NULL);\n>> if (!branch ||\n-- \nDuy\n"}]}