Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
If the new branch name to -b/-B happens to be missing, the next
argument may be mistaken as branch name and no longer recognized by
checkout as argument. This may lead to confusing error messages.
By checking branch name early and printing out the invalid name, users
may realize they forgot to specify branch name.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
On Tue, Aug 28, 2012 at 12:03 AM, Junio C Hamano gits...@pobox.com wrote:
Ideally you would want
fatal: -t is not an acceptable name for a branch
in this case; if it is cumbersome to arrange, at the very least,
updating paths is incompatible with checking out the branch -t.
would be clearer.
Fair enough. It turns out we do check branch name's validity. It's
just too late.
The surrounding code is somewhat tricky and the code structure is
brittle; there are places that update the opts.new_branch so the new
location of this check has to be after them, and there is one
codepath that having a bad value in it does not matter.
I had to check the code outside the context of this patch a few
times to convince myself that this patch does not break them. I'll
queue the patch as-is for now, but we probably would want to see how
we can structure it to be less brittle.
Thanks.
builtin/checkout.c | 20 ++--
t/t2018-checkout-branch.sh | 5 +
2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index d812219..03b0f25 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1049,6 +1049,16 @@ int cmd_checkout(int argc, const char **argv, const
char *prefix)
if (opts.track == BRANCH_TRACK_UNSPECIFIED)
opts.track = git_branch_track;
+ if (opts.new_branch) {
+ struct strbuf buf = STRBUF_INIT;
+
+ opts.branch_exists = validate_new_branchname(opts.new_branch,
buf,
+
!!opts.new_branch_force,
+
!!opts.new_branch_force);
+
+ strbuf_release(buf);
+ }
+
if (argc) {
const char **pathspec = get_pathspec(prefix, argv);
@@ -1079,16 +1089,6 @@ int cmd_checkout(int argc, const char **argv, const
char *prefix)
if (patch_mode)
return interactive_checkout(new.name, NULL, opts);
- if (opts.new_branch) {
- struct strbuf buf = STRBUF_INIT;
-
- opts.branch_exists = validate_new_branchname(opts.new_branch,
buf,
-
!!opts.new_branch_force,
-
!!opts.new_branch_force);
-
- strbuf_release(buf);
- }
-
if (new.name !new.commit) {
die(_(Cannot switch branch to a non-commit.));
}
diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 2741262..48ab6a2 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -198,4 +198,9 @@ test_expect_success 'checkout -B to the current branch
works' '
test_dirty_mergeable
'
+test_expect_success 'checkout -b checks branch validitity early' '
+ test_must_fail git checkout -b -t origin/master 2err
+ grep not a valid branch name err
+'
+
test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
