[PATCH 1/3] config.txt: a few lines about branch.<name>.description

[PATCH 1/3] config.txt: a few lines about branch.<name>.description

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index bf8f911..ee64846 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -735,6 +735,12 @@ branch.<name>.rebase::
 it unless you understand the implications (see linkgit:git-rebase[1]
 for details).
 
+branch.<name>.description::
+	Branch description, can be edited with
+	`git branch --edit-description`. Branch description is
+	automatically added in the format-patch cover letter or
+	request-pull summary.
+
 browser.<tool>.cmd::
 	Specify the command to invoke the specified browser. The
 	specified command is evaluated in shell with the URLs passed
-- 
1.8.0.rc2.23.g1fb49df

[PATCH 2/3] format-patch: pick up branch description when no ref is specified

[Nguyễn Thái Ngọc Duy]

find_branch_name() fails to detect "format-patch --cover-letter -3"
where no command line arguments are given and HEAD is automatically
added. Detect branch name in this case so we can pick up the branch's
description in cover letter.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/log.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index e7b7db1..115f118 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1027,8 +1027,22 @@ static char *find_branch_name(struct rev_info *rev)
 		else
 			return NULL;
 	}
-	if (positive < 0)
+	if (positive < 0) {
+		/*
+		 * No actual ref from command line, but "HEAD" from
+		 * rev->def was added in setup_revisions()
+		 * e.g. format-patch --cover-letter -12
+		 */
+		if (!rev->cmdline.nr &&
+		    rev->pending.nr == 1 &&
+		    !strcmp(rev->pending.objects[0].name, "HEAD")) {
+			const char *ref;
+			ref = resolve_ref_unsafe("HEAD", branch_sha1, 1, NULL);
+			if (ref && !prefixcmp(ref, "refs/heads/"))
+				return xstrdup(ref + strlen("refs/heads/"));
+		}
 		return NULL;
+	}
 	strbuf_addf(&buf, "refs/heads/%s", rev->cmdline.rev[positive].name);
 	branch = resolve_ref_unsafe(buf.buf, branch_sha1, 1, NULL);
 	if (!branch ||
-- 
1.8.0.rc2.23.g1fb49df

[PATCH 3/3] branch: delete branch description if it's empty

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 I don't see the value of having an empty branch description. But I
 may be missing something.

 builtin/branch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 1ec9c02..873f624 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -725,7 +725,7 @@ static int edit_branch_description(const char *branch_name)
 	stripspace(&buf, 1);
 
 	strbuf_addf(&name, "branch.%s.description", branch_name);
-	status = git_config_set(name.buf, buf.buf);
+	status = git_config_set(name.buf, buf.len ? buf.buf : NULL);
 	strbuf_release(&name);
 	strbuf_release(&buf);
 
-- 
1.8.0.rc2.23.g1fb49df

Re: [PATCH 2/3] format-patch: pick up branch description when no ref is specified

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> find_branch_name() fails to detect "format-patch --cover-letter -3"
> where no command line arguments are given and HEAD is automatically
> added.

Nicely spotted.

That is not the only case that takes this codepath, though.

    $ git format-patch --cover-letter master..

will also give you the same (if you say it without "..", which is
the more normal invocation of the command, then the caller already
know you meant the current branch and this function is not called).

And in that case you will have two tokens on cmdline.nr, one for
"master.."  to show where he bottom is, and the other for the
implied "HEAD"; I do not think this patch is a sufficient solution
for the more general cases, but on the other hand I do not know how
much it matters.

> -	if (positive < 0)
> +	if (positive < 0) {
> +		/*
> +		 * No actual ref from command line, but "HEAD" from
> +		 * rev->def was added in setup_revisions()
> +		 * e.g. format-patch --cover-letter -12
> +		 */

That comment does not describe "positive < 0" case, but belongs to
the conditional added in this patch, no?

> +		if (!rev->cmdline.nr &&
> +		    rev->pending.nr == 1 &&
> +		    !strcmp(rev->pending.objects[0].name, "HEAD")) {
> +			const char *ref;
> +			ref = resolve_ref_unsafe("HEAD", branch_sha1, 1, NULL);
> +			if (ref && !prefixcmp(ref, "refs/heads/"))
> +				return xstrdup(ref + strlen("refs/heads/"));
> +		}
>  		return NULL;
> +	}
>  	strbuf_addf(&buf, "refs/heads/%s", rev->cmdline.rev[positive].name);
>  	branch = resolve_ref_unsafe(buf.buf, branch_sha1, 1, NULL);
>  	if (!branch ||

Re: [PATCH 2/3] format-patch: pick up branch description when no ref is specified

[Duy Nguyen]

On Wed, Jan 2, 2013 at 3:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> find_branch_name() fails to detect "format-patch --cover-letter -3"
>> where no command line arguments are given and HEAD is automatically
>> added.
>
> Nicely spotted.
>
> That is not the only case that takes this codepath, though.
>
>     $ git format-patch --cover-letter master..
>
> will also give you the same (if you say it without "..", which is
> the more normal invocation of the command, then the caller already
> know you meant the current branch and this function is not called).
>
> And in that case you will have two tokens on cmdline.nr, one for
> "master.."  to show where he bottom is, and the other for the
> implied "HEAD";

Interesting. find_brach_name() handles this case wrong because
rev->cmdline[positive].name is "HEAD" and it goes ahead prepending
"refs/heads/" anyway. I'll fix it in the reroll. I was avoiding tests
with an excuse that you did not write tests when you added this
function either. But with this change, I think tests are required.

> I do not think this patch is a sufficient solution
> for the more general cases, but on the other hand I do not know how
> much it matters.

I think the best place to handle this is setup_revisions() for general
cases. But we do not need branch detection anywhere else yet, I guess
it's ok to fix it case by case here. We can move the code back to
revisions.c when there are more use cases for it.

>> -     if (positive < 0)
>> +     if (positive < 0) {
>> +             /*
>> +              * No actual ref from command line, but "HEAD" from
>> +              * rev->def was added in setup_revisions()
>> +              * e.g. format-patch --cover-letter -12
>> +              */
>
> That comment does not describe "positive < 0" case, but belongs to
> the conditional added in this patch, no?

It's meant as the comment for the following block, yes. I'll move it
into the block for clarity.

>> +             if (!rev->cmdline.nr &&
>> +                 rev->pending.nr == 1 &&
>> +                 !strcmp(rev->pending.objects[0].name, "HEAD")) {
>> +                     const char *ref;
>> +                     ref = resolve_ref_unsafe("HEAD", branch_sha1, 1, NULL);
>> +                     if (ref && !prefixcmp(ref, "refs/heads/"))
>> +                             return xstrdup(ref + strlen("refs/heads/"));
>> +             }
>>               return NULL;
>> +     }
>>       strbuf_addf(&buf, "refs/heads/%s", rev->cmdline.rev[positive].name);
>>       branch = resolve_ref_unsafe(buf.buf, branch_sha1, 1, NULL);
>>       if (!branch ||
-- 
Duy