[PATCH v3 16/18] parse_branchname_arg(): refactor the decision making

["Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@xxxxxxxxx>]

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@xxxxxxxxxxx>

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@xxxxxxxxxxx>
---
 builtin/checkout.c | 69 +++++++++++++++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 25 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 95a8e08793..9a85a3e4dc 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1153,9 +1153,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 arg0_cant_be_pathspec = 0;
-	int i;
+	int dash_dash_pos, i;
+	int recover_with_dwim, arg0_cant_be_pathspec;
 
 	/*
 	 * Resolve ambiguity where argv[0] may be <pathspec> or <commit>.
@@ -1174,14 +1173,6 @@ 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"));
-		arg0_cant_be_pathspec = 1;
-	}
 
 	arg = argv[0];
 	dash_dash_pos = -1;
@@ -1192,13 +1183,48 @@ 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)
-			arg0_cant_be_pathspec = 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.
+		 * Try to resolve it with additional knowledge about pathspec args.
+		 */
+		arg0_cant_be_pathspec = !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 (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> */
+		arg0_cant_be_pathspec = 1;
+	} else {
+		/* 'git checkout/switch/restore <commit> <unxpected> [...] -- [...]' */
+		die(_("only one reference expected, %d given."), dash_dash_pos);
 	}
 
 	opts->count_checkout_paths = !opts->quiet && !arg0_cant_be_pathspec;
@@ -1207,19 +1233,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 = !arg0_cant_be_pathspec &&
 			check_filename(opts->prefix, arg);
 
 		if (!arg0_cant_be_pathspec && !no_wildcard(arg))
 			recover_with_dwim = 0;
 
-		if (!(argc == 1 && dash_dash_pos == -1) &&
-		    !(argc == 2 && dash_dash_pos == 1) &&
-		    opts->accept_pathspec)
-			recover_with_dwim = 0;
-
 		if (recover_with_dwim) {
 			const char *remote = parse_remote_branch(arg, rev,
 								 could_be_checkout_paths);
-- 
gitgitgadget