[PATCH] branch: do not fail a no-op --edit-desc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



In a repository on a branch without branch description, try running
"git branch --edit-description" and then exit the editor after
emptying the edit buffer, which is the way to tell the command that
you changed your mind and you do not want the description after all.

The command should just happily oblige, adding no branch description
for the current branch, and exit successfully.  But it fails to do
so:

    $ git init -b main
    $ git commit --allow-empty -m commit
    $ GIT_EDITOR=: git branch --edit-description
    fatal: could not unset 'branch.main.description'

The end result is OK in that the configuration variable does not
exist in the resulting repository, but we should do better.

One way to solve this is to replace the git_config_set() call that
is also used to unset the variable when the edited buffer is empty
with git_config_set_gently(), so that we do not consider it an error
that we fail to unset something that does not exist in the first
place.

But there is even a simpler way, which is to take advantage of the
fact that we _know_ if the variable existed in the first place.  If
we know we didn't have description, and if we are asked not to have
a description by the editor, we can just return doing nothing.

The simpler solution of course introduces TOCTOU, but you are
fooling yourself in your own repository.  Not overwriting the branch
description on the same branch you added in another window, while
you had this other editor open, may even be a feature ;-)

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
 builtin/branch.c  | 6 ++++--
 t/t3200-branch.sh | 3 +++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git c/builtin/branch.c w/builtin/branch.c
index 5d00d0b8d3..dcd847158a 100644
--- c/builtin/branch.c
+++ w/builtin/branch.c
@@ -604,10 +604,11 @@ static GIT_PATH_FUNC(edit_description, "EDIT_DESCRIPTION")
 
 static int edit_branch_description(const char *branch_name)
 {
+	int exists;
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf name = STRBUF_INIT;
 
-	read_branch_desc(&buf, branch_name);
+	exists = !read_branch_desc(&buf, branch_name);
 	if (!buf.len || buf.buf[buf.len-1] != '\n')
 		strbuf_addch(&buf, '\n');
 	strbuf_commented_addf(&buf,
@@ -624,7 +625,8 @@ static int edit_branch_description(const char *branch_name)
 	strbuf_stripspace(&buf, 1);
 
 	strbuf_addf(&name, "branch.%s.description", branch_name);
-	git_config_set(name.buf, buf.len ? buf.buf : NULL);
+	if (buf.len || exists)
+		git_config_set(name.buf, buf.len ? buf.buf : NULL);
 	strbuf_release(&name);
 	strbuf_release(&buf);
 
diff --git c/t/t3200-branch.sh w/t/t3200-branch.sh
index 9723c2827c..5f72fd7453 100755
--- c/t/t3200-branch.sh
+++ w/t/t3200-branch.sh
@@ -1381,6 +1381,9 @@ test_expect_success 'branch --delete --force removes dangling branch' '
 '
 
 test_expect_success 'use --edit-description' '
+	EDITOR=: git branch --edit-description &&
+	test_must_fail git config branch.main.description &&
+
 	write_script editor <<-\EOF &&
 		echo "New contents" >"$1"
 	EOF



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux