[PATCH v3 15/18] parse_branchname_arg(): update code comments

From

AGAlexandr Miloslavskiy via GitGitGadget <gitgitgadget@gmail.com>

Date

Dec 19, 2019, 18:01 UTC

Message-ID

<7989f0c5cf813010d8034e932ee78959c7b39c95.1576778515.git.gitgitgadget@gmail.com>

In-Reply-To

<pull.490.v3.git.1576778515.gitgitgadget@gmail.com>

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

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

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 63f4bb4da6..95a8e08793 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1158,45 +1158,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;
@@ -1218,9 +1194,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)
-			arg0_cant_be_pathspec = 1; /* case (3) or (1) */
+			arg0_cant_be_pathspec = 1;
 		else if (dash_dash_pos >= 2)
 			die(_("only one reference expected, %d given."), dash_dash_pos);
 	}
@@ -1231,14 +1207,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 = !arg0_cant_be_pathspec &&
@@ -1247,10 +1215,6 @@ static int parse_branchname_arg(int argc, const char **argv,
 		if (!arg0_cant_be_pathspec && !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 && dash_dash_pos == -1) &&
 		    !(argc == 2 && dash_dash_pos == 1) &&
 		    opts->accept_pathspec)
@@ -1262,7 +1226,7 @@ static int parse_branchname_arg(int argc, const char **argv,
 			if (remote) {
 				*new_branch = arg;
 				arg = remote;
-				/* DWIMmed to create local branch, case (3).(b) */
+				/* DWIMmed to create local branch */
 			} else {
 				recover_with_dwim = 0;
 			}
@@ -1277,19 +1241,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 (!arg0_cant_be_pathspec) {	/* 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 (!arg0_cant_be_pathspec && argc > 1)
+		verify_non_filename(opts->prefix, arg);
 
 	return (opts->accept_pathspec && dash_dash_pos == 1) ? 2 : 1;
 }

-- 
gitgitgadget