git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
* [PATCH 0/5] parse_branchname_arg(): make code easier to understand
@ 2019-11-28 19:32 Alexandr Miloslavskiy via GitGitGadget
  2019-11-28 19:32 ` [PATCH 1/5] parse_branchname_arg(): fix dash_dash_pos, drop argcount Alexandr Miloslavskiy via GitGitGadget
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-11-28 19:32 UTC (permalink / raw)
  To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano

My bigger goal is to complete my --pathspec-from-file series of patches.

For this, I needed to evaluate whether parse_branchname_arg() heuristics
needs any changes when pathspec is passed, but NOT in args.

I found it surprisingly hard to reason about the code in this function. This
mostly happens due to "obfuscated" variables, where they have a clear name
and a different actual meaning. Ultimately, it was hard to mentally expand
them to true meaning AND see all possible combinations of branches at once.

I have split this refactoring in 4 patches, so that diffs are not too big in
every single patch.

To my understanding, there should be no changes in git's behavior, except
for a couple better die() messages.

Alexandr Miloslavskiy (5):
  parse_branchname_arg(): fix dash_dash_pos, drop argcount
  parse_branchname_arg(): introduce expect_commit_only
  parse_branchname_arg(): update code comments
  parse_branchname_arg(): refactor the decision making
  t2024: cover more cases

 builtin/checkout.c       | 174 ++++++++++++++++-----------------------
 t/t2024-checkout-dwim.sh |  27 ++++++
 2 files changed, 97 insertions(+), 104 deletions(-)


base-commit: d9f6f3b6195a0ca35642561e530798ad1469bd41
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-479%2FSyntevoAlex%2F%230225(git)_refactor_parse_branchname_arg-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-479/SyntevoAlex/#0225(git)_refactor_parse_branchname_arg-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/479
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/5] parse_branchname_arg(): fix dash_dash_pos, drop argcount
  2019-11-28 19:32 [PATCH 0/5] parse_branchname_arg(): make code easier to understand Alexandr Miloslavskiy via GitGitGadget
@ 2019-11-28 19:32 ` Alexandr Miloslavskiy via GitGitGadget
  2019-11-28 19:32 ` [PATCH 2/5] parse_branchname_arg(): introduce expect_commit_only Alexandr Miloslavskiy via GitGitGadget
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-11-28 19:32 UTC (permalink / raw)
  To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

`dash_dash_pos` was only calculated under `opts->accept_pathspec`. This
is unexpected to readers and made it harder to reason about the code.
Fix this by restoring the expected meaning.

Simplify the code by dropping `argcount` and useless `argc` / `argv`
manipulations.

This should not change behavior in any way.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 builtin/checkout.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3634a3dac1..655b389756 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1121,7 +1121,6 @@ static int parse_branchname_arg(int argc, const char **argv,
 				int *dwim_remotes_matched)
 {
 	const char **new_branch = &opts->new_branch;
-	int argcount = 0;
 	const char *arg;
 	int dash_dash_pos;
 	int has_dash_dash = 0;
@@ -1180,17 +1179,21 @@ static int parse_branchname_arg(int argc, const char **argv,
 	arg = argv[0];
 	dash_dash_pos = -1;
 	for (i = 0; i < argc; i++) {
-		if (opts->accept_pathspec && !strcmp(argv[i], "--")) {
+		if (!strcmp(argv[i], "--")) {
 			dash_dash_pos = i;
 			break;
 		}
 	}
-	if (dash_dash_pos == 0)
-		return 1; /* case (2) */
-	else if (dash_dash_pos == 1)
-		has_dash_dash = 1; /* case (3) or (1) */
-	else if (dash_dash_pos >= 2)
-		die(_("only one reference expected, %d given."), dash_dash_pos);
+
+	if (opts->accept_pathspec) {
+	    if (dash_dash_pos == 0)
+		    return 1; /* case (2) */
+	    else if (dash_dash_pos == 1)
+		    has_dash_dash = 1; /* case (3) or (1) */
+	    else if (dash_dash_pos >= 2)
+		    die(_("only one reference expected, %d given."), dash_dash_pos);
+	}
+
 	opts->count_checkout_paths = !opts->quiet && !has_dash_dash;
 
 	if (!strcmp(arg, "-"))
@@ -1241,15 +1244,10 @@ static int parse_branchname_arg(int argc, const char **argv,
 		if (!recover_with_dwim) {
 			if (has_dash_dash)
 				die(_("invalid reference: %s"), arg);
-			return argcount;
+			return 0;
 		}
 	}
 
-	/* we can't end up being in (2) anymore, eat the argument */
-	argcount++;
-	argv++;
-	argc--;
-
 	setup_new_branch_info_and_source_tree(new_branch_info, opts, rev, arg);
 
 	if (!opts->source_tree)                   /* case (1): want a tree */
@@ -1262,15 +1260,11 @@ static int parse_branchname_arg(int argc, const char **argv,
 		 * even if there happen to be a file called 'branch';
 		 * it would be extremely annoying.
 		 */
-		if (argc)
+		if (argc > 1)
 			verify_non_filename(opts->prefix, arg);
-	} else if (opts->accept_pathspec) {
-		argcount++;
-		argv++;
-		argc--;
 	}
 
-	return argcount;
+	return (dash_dash_pos == 1) ? 2 : 1;
 }
 
 static int switch_unborn_to_new_branch(const struct checkout_opts *opts)
-- 
gitgitgadget


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/5] parse_branchname_arg(): introduce expect_commit_only
  2019-11-28 19:32 [PATCH 0/5] parse_branchname_arg(): make code easier to understand Alexandr Miloslavskiy via GitGitGadget
  2019-11-28 19:32 ` [PATCH 1/5] parse_branchname_arg(): fix dash_dash_pos, drop argcount Alexandr Miloslavskiy via GitGitGadget
@ 2019-11-28 19:32 ` Alexandr Miloslavskiy via GitGitGadget
  2019-11-28 19:32 ` [PATCH 3/5] parse_branchname_arg(): update code comments Alexandr Miloslavskiy via GitGitGadget
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-11-28 19:32 UTC (permalink / raw)
  To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

`has_dash_dash` unexpectedly takes `opts->accept_pathspec` into account.
While this may sound clever at first sight, it becomes pretty hard to
reason (and not be a victim) about code, especially in combination with
`argc` here:

	if (!(argc == 1 && !has_dash_dash) &&
	    !(argc == 2 && has_dash_dash) &&
	    opts->accept_pathspec)
		recover_with_dwim = 0;

Introduce a new non-obfuscated variable to reduce the amount of diffs in
next patch.

This should not change behavior in any way.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 builtin/checkout.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 655b389756..5c6131dbe6 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1123,7 +1123,7 @@ static int parse_branchname_arg(int argc, const char **argv,
 	const char **new_branch = &opts->new_branch;
 	const char *arg;
 	int dash_dash_pos;
-	int has_dash_dash = 0;
+	int has_dash_dash = 0, expect_commit_only = 0;
 	int i;
 
 	/*
@@ -1194,7 +1194,10 @@ static int parse_branchname_arg(int argc, const char **argv,
 		    die(_("only one reference expected, %d given."), dash_dash_pos);
 	}
 
-	opts->count_checkout_paths = !opts->quiet && !has_dash_dash;
+	if (has_dash_dash)
+	    expect_commit_only = 1;
+
+	opts->count_checkout_paths = !opts->quiet && !expect_commit_only;
 
 	if (!strcmp(arg, "-"))
 		arg = "@{-1}";
@@ -1210,10 +1213,10 @@ static int parse_branchname_arg(int argc, const char **argv,
 		 */
 		int recover_with_dwim = dwim_new_local_branch_ok;
 
-		int could_be_checkout_paths = !has_dash_dash &&
+		int could_be_checkout_paths = !expect_commit_only &&
 			check_filename(opts->prefix, arg);
 
-		if (!has_dash_dash && !no_wildcard(arg))
+		if (!expect_commit_only && !no_wildcard(arg))
 			recover_with_dwim = 0;
 
 		/*
@@ -1242,7 +1245,7 @@ static int parse_branchname_arg(int argc, const char **argv,
 		}
 
 		if (!recover_with_dwim) {
-			if (has_dash_dash)
+			if (expect_commit_only)
 				die(_("invalid reference: %s"), arg);
 			return 0;
 		}
@@ -1253,7 +1256,7 @@ static int parse_branchname_arg(int argc, const char **argv,
 	if (!opts->source_tree)                   /* case (1): want a tree */
 		die(_("reference is not a tree: %s"), arg);
 
-	if (!has_dash_dash) {	/* case (3).(d) -> (1) */
+	if (!expect_commit_only) {	/* case (3).(d) -> (1) */
 		/*
 		 * Do not complain the most common case
 		 *	git checkout branch
-- 
gitgitgadget


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/5] parse_branchname_arg(): update code comments
  2019-11-28 19:32 [PATCH 0/5] parse_branchname_arg(): make code easier to understand Alexandr Miloslavskiy via GitGitGadget
  2019-11-28 19:32 ` [PATCH 1/5] parse_branchname_arg(): fix dash_dash_pos, drop argcount Alexandr Miloslavskiy via GitGitGadget
  2019-11-28 19:32 ` [PATCH 2/5] parse_branchname_arg(): introduce expect_commit_only Alexandr Miloslavskiy via GitGitGadget
@ 2019-11-28 19:32 ` Alexandr Miloslavskiy via GitGitGadget
  2019-11-28 19:32 ` [PATCH 4/5] parse_branchname_arg(): refactor the decision making Alexandr Miloslavskiy via GitGitGadget
  2019-11-28 19:32 ` [PATCH 5/5] t2024: cover more cases Alexandr Miloslavskiy via GitGitGadget
  4 siblings, 0 replies; 6+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-11-28 19:32 UTC (permalink / raw)
  To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

These parts repeat git documentation:
    ... if <something> is A...B <...>
    ... remote named in checkout.defaultRemote ...

Some parts repeat the code below. With next patch, code will be easier
to understand, so this is no longer needed.

This is a separate patch to reduce the amount of diffs in next patch.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 builtin/checkout.c | 86 +++++++++++-----------------------------------
 1 file changed, 21 insertions(+), 65 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5c6131dbe6..723aaca0ef 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1127,45 +1127,21 @@ static int parse_branchname_arg(int argc, const char **argv,
 	int i;
 
 	/*
-	 * case 1: git checkout <ref> -- [<paths>]
-	 *
-	 *   <ref> must be a valid tree, everything after the '--' must be
-	 *   a path.
-	 *
-	 * case 2: git checkout -- [<paths>]
-	 *
-	 *   everything after the '--' must be paths.
-	 *
-	 * case 3: git checkout <something> [--]
-	 *
-	 *   (a) If <something> is a commit, that is to
-	 *       switch to the branch or detach HEAD at it.  As a special case,
-	 *       if <something> is A...B (missing A or B means HEAD but you can
-	 *       omit at most one side), and if there is a unique merge base
-	 *       between A and B, A...B names that merge base.
-	 *
-	 *   (b) If <something> is _not_ a commit, either "--" is present
-	 *       or <something> is not a path, no -t or -b was given, and
-	 *       and there is a tracking branch whose name is <something>
-	 *       in one and only one remote (or if the branch exists on the
-	 *       remote named in checkout.defaultRemote), then this is a
-	 *       short-hand to fork local <something> from that
-	 *       remote-tracking branch.
-	 *
-	 *   (c) Otherwise, if "--" is present, treat it like case (1).
-	 *
-	 *   (d) Otherwise :
-	 *       - if it's a reference, treat it like case (1)
-	 *       - else if it's a path, treat it like case (2)
-	 *       - else: fail.
-	 *
-	 * case 4: git checkout <something> <paths>
-	 *
-	 *   The first argument must not be ambiguous.
-	 *   - If it's *only* a reference, treat it like case (1).
-	 *   - If it's only a path, treat it like case (2).
-	 *   - else: fail.
-	 *
+	 * Resolve ambiguity where argv[0] may be <pathspec> or <commit>.
+	 * High-level approach is:
+	 * 1) Use various things to reduce ambiguity, examples:
+	 *    * '--' is present
+	 *    * command doesn't accept <pathspec>
+	 *    * additional options like '-b' were given
+	 * 2) If ambiguous and matches both existing <commit> and existing
+	 *    file, complain. However, in 1-argument 'git checkout <arg>'
+	 *    treat as <commit> to avoid annoying users.
+	 * 3) Otherwise, if it matches some existing <commit>, treat as
+	 *    <commit>.
+	 * 4) Otherwise, if it matches a remote branch, and it's considered
+	 *    reasonable to DWIM to create a local branch from remote branch,
+	 *    do that and proceed with (2)(3).
+	 * 5) Otherwise, let caller proceed with <pathspec> interpretation.
 	 */
 	if (!argc)
 		return 0;
@@ -1187,9 +1163,9 @@ static int parse_branchname_arg(int argc, const char **argv,
 
 	if (opts->accept_pathspec) {
 	    if (dash_dash_pos == 0)
-		    return 1; /* case (2) */
+		    return 1;
 	    else if (dash_dash_pos == 1)
-		    has_dash_dash = 1; /* case (3) or (1) */
+		    has_dash_dash = 1;
 	    else if (dash_dash_pos >= 2)
 		    die(_("only one reference expected, %d given."), dash_dash_pos);
 	}
@@ -1203,14 +1179,6 @@ static int parse_branchname_arg(int argc, const char **argv,
 		arg = "@{-1}";
 
 	if (get_oid_mb(arg, rev)) {
-		/*
-		 * Either case (3) or (4), with <something> not being
-		 * a commit, or an attempt to use case (1) with an
-		 * invalid ref.
-		 *
-		 * It's likely an error, but we need to find out if
-		 * we should auto-create the branch, case (3).(b).
-		 */
 		int recover_with_dwim = dwim_new_local_branch_ok;
 
 		int could_be_checkout_paths = !expect_commit_only &&
@@ -1219,10 +1187,6 @@ static int parse_branchname_arg(int argc, const char **argv,
 		if (!expect_commit_only && !no_wildcard(arg))
 			recover_with_dwim = 0;
 
-		/*
-		 * Accept "git checkout foo", "git checkout foo --"
-		 * and "git switch foo" as candidates for dwim.
-		 */
 		if (!(argc == 1 && !has_dash_dash) &&
 		    !(argc == 2 && has_dash_dash) &&
 		    opts->accept_pathspec)
@@ -1238,7 +1202,7 @@ static int parse_branchname_arg(int argc, const char **argv,
 					    arg);
 				*new_branch = arg;
 				arg = remote;
-				/* DWIMmed to create local branch, case (3).(b) */
+				/* DWIMmed to create local branch */
 			} else {
 				recover_with_dwim = 0;
 			}
@@ -1253,19 +1217,11 @@ static int parse_branchname_arg(int argc, const char **argv,
 
 	setup_new_branch_info_and_source_tree(new_branch_info, opts, rev, arg);
 
-	if (!opts->source_tree)                   /* case (1): want a tree */
+	if (!opts->source_tree)
 		die(_("reference is not a tree: %s"), arg);
 
-	if (!expect_commit_only) {	/* case (3).(d) -> (1) */
-		/*
-		 * Do not complain the most common case
-		 *	git checkout branch
-		 * even if there happen to be a file called 'branch';
-		 * it would be extremely annoying.
-		 */
-		if (argc > 1)
-			verify_non_filename(opts->prefix, arg);
-	}
+	if (!expect_commit_only && argc > 1)
+		verify_non_filename(opts->prefix, arg);
 
 	return (dash_dash_pos == 1) ? 2 : 1;
 }
-- 
gitgitgadget


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 4/5] parse_branchname_arg(): refactor the decision making
  2019-11-28 19:32 [PATCH 0/5] parse_branchname_arg(): make code easier to understand Alexandr Miloslavskiy via GitGitGadget
                   ` (2 preceding siblings ...)
  2019-11-28 19:32 ` [PATCH 3/5] parse_branchname_arg(): update code comments Alexandr Miloslavskiy via GitGitGadget
@ 2019-11-28 19:32 ` Alexandr Miloslavskiy via GitGitGadget
  2019-11-28 19:32 ` [PATCH 5/5] t2024: cover more cases Alexandr Miloslavskiy via GitGitGadget
  4 siblings, 0 replies; 6+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-11-28 19:32 UTC (permalink / raw)
  To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

Make it easier to understand which branches handle which cases.

Drop obfuscated variable `has_dash_dash` which also took
`opts->accept_pathspec` into account, making it pretty hard to reason
about code, especially when used together with `argc` and
`opts->accept_pathspec` here:

	if (!(argc == 1 && !has_dash_dash) &&
		!(argc == 2 && has_dash_dash) &&
		opts->accept_pathspec)
		recover_with_dwim = 0;

Avoid double-negation in the code mentioned above ("it is not OK to
proceed if it's not one of those cases").

Avoid hard-to-understand condition `opts->accept_pathspec` in the code
mentioned above.

There are some minor die() message changes for:
`git switch <commit> <unexpected>`
`git switch <commit> -- <unexpected>`

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 builtin/checkout.c | 71 +++++++++++++++++++++++++++-------------------
 1 file changed, 42 insertions(+), 29 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 723aaca0ef..8f679d1b6a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1122,9 +1122,8 @@ static int parse_branchname_arg(int argc, const char **argv,
 {
 	const char **new_branch = &opts->new_branch;
 	const char *arg;
-	int dash_dash_pos;
-	int has_dash_dash = 0, expect_commit_only = 0;
-	int i;
+	int dash_dash_pos, i;
+	int recover_with_dwim, expect_commit_only;
 
 	/*
 	 * Resolve ambiguity where argv[0] may be <pathspec> or <commit>.
@@ -1143,15 +1142,7 @@ static int parse_branchname_arg(int argc, const char **argv,
 	 *    do that and proceed with (2)(3).
 	 * 5) Otherwise, let caller proceed with <pathspec> interpretation.
 	 */
-	if (!argc)
-		return 0;
-
-	if (!opts->accept_pathspec) {
-		if (argc > 1)
-			die(_("only one reference expected"));
-		has_dash_dash = 1; /* helps disambiguate */
-	}
-
+	 
 	arg = argv[0];
 	dash_dash_pos = -1;
 	for (i = 0; i < argc; i++) {
@@ -1161,17 +1152,46 @@ static int parse_branchname_arg(int argc, const char **argv,
 		}
 	}
 
-	if (opts->accept_pathspec) {
-	    if (dash_dash_pos == 0)
-		    return 1;
-	    else if (dash_dash_pos == 1)
-		    has_dash_dash = 1;
-	    else if (dash_dash_pos >= 2)
-		    die(_("only one reference expected, %d given."), dash_dash_pos);
-	}
+	if (dash_dash_pos == -1) {
+		if (argc == 0) {
+			/* 'git checkout/switch/restore' */
+			return 0;
+		} else if (argc == 1) {
+			/* 'git checkout/switch/restore <something>' */
+			recover_with_dwim = dwim_new_local_branch_ok;
+		} else if (!opts->accept_pathspec) {
+			/* 'git switch <commit> <unexpected> [...]' */
+			die(_("only one reference expected, %d given."), argc);
+		} else {
+			/* 'git checkout/restore <something> <pathspec> [...]' */
+			recover_with_dwim = 0;
+		}
+
+		/* Absence of '--' leaves <pathspec>/<commit> ambiguity */
+		expect_commit_only = !opts->accept_pathspec;
+	} else if (dash_dash_pos == 0) {
+		/* 'git checkout/switch/restore -- [...]' */
+		return 1;  /* Eat '--' */
+	} else if (dash_dash_pos == 1) {
+		if (!opts->accept_pathspec) {
+			/* 'git switch <commit> -- [...]' */
+			die(_("incompatible with pathspec arguments"));
+		}
 
-	if (has_dash_dash)
-	    expect_commit_only = 1;
+		if (argc == 2) {
+			/* 'git checkout/restore <commit> --' */
+			recover_with_dwim = dwim_new_local_branch_ok;
+		} else {
+			/* 'git checkout/restore <commit> -- <pathspec> [...]' */
+			recover_with_dwim = 0;
+		}
+
+		/* Presence of '--' makes it certain that arg is <commit> */
+		expect_commit_only = 1;
+	} else {
+		/* 'git checkout/switch/restore <commit> <unxpected> [...] -- [...]' */
+		die(_("only one reference expected, %d given."), dash_dash_pos);
+	}
 
 	opts->count_checkout_paths = !opts->quiet && !expect_commit_only;
 
@@ -1179,19 +1199,12 @@ static int parse_branchname_arg(int argc, const char **argv,
 		arg = "@{-1}";
 
 	if (get_oid_mb(arg, rev)) {
-		int recover_with_dwim = dwim_new_local_branch_ok;
-
 		int could_be_checkout_paths = !expect_commit_only &&
 			check_filename(opts->prefix, arg);
 
 		if (!expect_commit_only && !no_wildcard(arg))
 			recover_with_dwim = 0;
 
-		if (!(argc == 1 && !has_dash_dash) &&
-		    !(argc == 2 && has_dash_dash) &&
-		    opts->accept_pathspec)
-			recover_with_dwim = 0;
-
 		if (recover_with_dwim) {
 			const char *remote = unique_tracking_name(arg, rev,
 								  dwim_remotes_matched);
-- 
gitgitgadget


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 5/5] t2024: cover more cases
  2019-11-28 19:32 [PATCH 0/5] parse_branchname_arg(): make code easier to understand Alexandr Miloslavskiy via GitGitGadget
                   ` (3 preceding siblings ...)
  2019-11-28 19:32 ` [PATCH 4/5] parse_branchname_arg(): refactor the decision making Alexandr Miloslavskiy via GitGitGadget
@ 2019-11-28 19:32 ` Alexandr Miloslavskiy via GitGitGadget
  4 siblings, 0 replies; 6+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-11-28 19:32 UTC (permalink / raw)
  To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 t/t2024-checkout-dwim.sh | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index fa0718c730..2fe77387a6 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -132,6 +132,33 @@ test_expect_success 'checkout of branch from a single remote succeeds #2' '
 	test_branch_upstream baz repo_b baz
 '
 
+test_expect_success 'checkout of branch from a single remote succeeds with --' '
+	git checkout -B master &&
+	test_might_fail git branch -D baz &&
+
+	git checkout baz -- &&
+	status_uno_is_clean &&
+	test_branch baz &&
+	test_cmp_rev remotes/other_b/baz HEAD &&
+	test_branch_upstream baz repo_b baz
+'
+
+test_expect_success 'dont DWIM with pathspec #1' '
+	git checkout -B master &&
+	test_might_fail git branch -D baz &&
+
+	test_must_fail git checkout baz nonExistingFile 2>err &&
+	test_i18ngrep "did not match any file(s) known to git" err
+'
+
+test_expect_success 'dont DWIM with pathspec #2' '
+	git checkout -B master &&
+	test_might_fail git branch -D baz &&
+
+	test_must_fail git checkout baz -- nonExistingFile 2>err &&
+	test_i18ngrep "fatal: invalid reference: baz" err
+'
+
 test_expect_success '--no-guess suppresses branch auto-vivification' '
 	git checkout -B master &&
 	status_uno_is_clean &&
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-28 19:32 [PATCH 0/5] parse_branchname_arg(): make code easier to understand Alexandr Miloslavskiy via GitGitGadget
2019-11-28 19:32 ` [PATCH 1/5] parse_branchname_arg(): fix dash_dash_pos, drop argcount Alexandr Miloslavskiy via GitGitGadget
2019-11-28 19:32 ` [PATCH 2/5] parse_branchname_arg(): introduce expect_commit_only Alexandr Miloslavskiy via GitGitGadget
2019-11-28 19:32 ` [PATCH 3/5] parse_branchname_arg(): update code comments Alexandr Miloslavskiy via GitGitGadget
2019-11-28 19:32 ` [PATCH 4/5] parse_branchname_arg(): refactor the decision making Alexandr Miloslavskiy via GitGitGadget
2019-11-28 19:32 ` [PATCH 5/5] t2024: cover more cases Alexandr Miloslavskiy via GitGitGadget

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git