git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Michael Haggerty <mhagger@alum.mit.edu>, Jeff King <peff@peff.net>
Subject: [PATCH] branch --edit-description: protect against mistyped branch name
Date: Sun, 05 Feb 2012 17:26:31 -0800	[thread overview]
Message-ID: <7vaa4wda60.fsf_-_@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20120130214842.GA16149@sigill.intra.peff.net> (Jeff King's message of "Mon, 30 Jan 2012 16:48:43 -0500")

It is very easy to mistype the branch name when editing its description,
e.g.

	$ git checkout -b my-topic master
	: work work work
	: now we are at a good point to switch working something else
	$ git checkout master
	: ah, let's write it down before we forget what we were doing
	$ git branch --edit-description my-tpoic

The command does not notice that branch 'my-tpoic' does not exist.  It is
not lost (it becomes description of an unborn my-tpoic branch), but is not
very useful.  So detect such a case and error out to reduce the grief
factor from this common mistake.

This incidentally also errors out --edit-description when the HEAD points
at an unborn branch (immediately after "init", or "checkout --orphan"),
because at that point, you do not even have any commit that is part of
your history and there is no point in describing how this particular
branch is different from the branch it forked off of, which is the useful
bit of information the branch description is designed to capture.

We may want to special case the unborn case later, but that is outside the
scope of this patch to prevent more common mistakes before 1.7.9 series
gains too much widespread use.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

  Jeff King <peff@peff.net> writes:

  > IOW, the problem with the current code is that it allows typos and other
  > arbitrary bogus names to be silently described, even though doing so is
  > probably an error...

 builtin/branch.c  |   15 +++++++++++++++
 t/t3200-branch.sh |   41 +++++++++++++++++++++++++++++++++++++----
 2 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 7095718..0c1784f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -768,6 +768,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 				      with_commit, argv);
 	else if (edit_description) {
 		const char *branch_name;
+		struct strbuf branch_ref = STRBUF_INIT;
+
 		if (detached)
 			die("Cannot give description to detached HEAD");
 		if (!argc)
@@ -776,6 +778,19 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			branch_name = argv[0];
 		else
 			usage_with_options(builtin_branch_usage, options);
+
+		strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
+		if (!ref_exists(branch_ref.buf)) {
+			strbuf_reset(&branch_ref);
+
+			if (!argc)
+				return error("No commit on branch '%s' yet.",
+					     branch_name);
+			else
+				return error("No such branch '%s'.", branch_name);
+		}
+		strbuf_reset(&branch_ref);
+
 		if (edit_branch_description(branch_name))
 			return 1;
 	} else if (rename) {
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index ea82424..dd1aceb 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -3,11 +3,8 @@
 # Copyright (c) 2005 Amos Waterland
 #
 
-test_description='git branch --foo should not create bogus branch
+test_description='git branch assorted tests'
 
-This test runs git branch --help and checks that the argument is properly
-handled.  Specifically, that a bogus branch is not created.
-'
 . ./test-lib.sh
 
 test_expect_success \
@@ -620,4 +617,40 @@ test_expect_success 'use set-upstream on the current branch' '
 
 '
 
+test_expect_success 'use --edit-description' '
+	write_script editor <<-\EOF &&
+		echo "New contents" >"$1"
+	EOF
+	EDITOR=./editor git branch --edit-description &&
+		write_script editor <<-\EOF &&
+		git stripspace -s <"$1" >"EDITOR_OUTPUT"
+	EOF
+	EDITOR=./editor git branch --edit-description &&
+	echo "New contents" >expect &&
+	test_cmp EDITOR_OUTPUT expect
+'
+
+test_expect_success 'detect typo in branch name when using --edit-description' '
+	write_script editor <<-\EOF &&
+		echo "New contents" >"$1"
+	EOF
+	(
+		EDITOR=./editor &&
+		export EDITOR &&
+		test_must_fail git branch --edit-description no-such-branch
+	)
+'
+
+test_expect_success 'refuse --edit-description on unborn branch for now' '
+	write_script editor <<-\EOF &&
+		echo "New contents" >"$1"
+	EOF
+	git checkout --orphan unborn &&
+	(
+		EDITOR=./editor &&
+		export EDITOR &&
+		test_must_fail git branch --edit-description
+	)
+'
+
 test_done
-- 
1.7.9.172.ge26ae

  reply	other threads:[~2012-02-06  1:26 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-29  6:09 Bug: "git checkout -b" should be allowed in empty repo Michael Haggerty
2012-01-29  6:56 ` Junio C Hamano
2012-01-29  7:50   ` Junio C Hamano
2012-01-30  6:38   ` Michael Haggerty
2012-01-30 18:48     ` Junio C Hamano
2012-01-30 20:10       ` Junio C Hamano
2012-01-30 21:50         ` Jeff King
2012-02-06  2:06           ` Junio C Hamano
2012-02-06  2:08             ` Junio C Hamano
2012-02-06  4:30             ` Jeff King
2012-02-06  4:42               ` Andrew Ardill
2012-02-06  5:06                 ` Jeff King
2012-02-06  8:51                   ` Michael Haggerty
2012-02-06  8:57                     ` Jeff King
2012-02-06 18:17                       ` Junio C Hamano
2012-02-06 20:14                         ` Jeff King
2012-02-07  8:04                         ` Michael Haggerty
2012-02-06 18:39                 ` demerphq
2012-02-06  5:15               ` Junio C Hamano
2012-02-06  5:18                 ` Jeff King
2012-02-06  5:30                   ` Junio C Hamano
2012-02-06  5:34                     ` Junio C Hamano
2012-02-06  5:45                     ` Jeff King
2012-02-06  8:59                     ` Michael Haggerty
2012-02-06 18:31                       ` Junio C Hamano
2012-01-30 21:48       ` Jeff King
2012-02-06  1:26         ` Junio C Hamano [this message]
2012-02-06  1:27           ` [PATCH] branch --edit-description: protect against mistyped branch name Junio C Hamano
2012-02-06  4:20           ` Jeff King
2012-01-31  8:57       ` Bug: "git checkout -b" should be allowed in empty repo Michael Haggerty
2012-01-31 10:01         ` Johannes Sixt
2012-01-31 10:11           ` demerphq
2012-01-31 10:09         ` Jakub Narebski
2012-01-31 16:32           ` Michael Haggerty

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vaa4wda60.fsf_-_@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).