Re: [PATCH 1/3] checkout.c: add strict usage of -- before file_path

[Duy Nguyen <pclouds@gmail.com>]

On Mon, May 14, 2018 at 04:52:53PM +0200, Duy Nguyen wrote:
> On Sun, May 13, 2018 at 11:02 PM, Kevin Daudt <me@ikke.info> wrote:
> > One data point indicating this is giving issues is that today on IRC a
> > user was confused why `git checkout pt` did not show any message and did
> > not checkout a remote branch called 'pt' as they expected. It turned out
> > they also had a local file/dir called 'pt', which caused git to checkout
> > that file/dir rather than creating a local branch based on the remote
> > branch.
> 
> Now this is something we should fix. When an argument can be
> interpreted in more than one way Git should reject it, but I think
> this ambiguation logic does not take dwim (i.e. create a new branch
> beased on remote) into account.

This is a quick draft to make this happen

-- 8< --
diff --git a/builtin/checkout.c b/builtin/checkout.c
index b49b582071..f4f6951f05 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -953,8 +953,19 @@ static int parse_branchname_arg(int argc, const char **argv,
 		 */
 		int recover_with_dwim = dwim_new_local_branch_ok;
 
-		if (!has_dash_dash &&
-		    (check_filename(opts->prefix, arg) || !no_wildcard(arg)))
+		if (!has_dash_dash && check_filename(opts->prefix, arg) &&
+		    recover_with_dwim) {
+			const char *remote = unique_tracking_name(arg, rev);
+			if (remote)
+				die(_("don't know whether to create a tracking "
+				      "branch from remote %s or to checkout path %s, "
+				      "use -- to disambiguate"),
+				    remote, arg);
+			printf("here?\n");
+			recover_with_dwim = 0;
+		}
+
+		if (!has_dash_dash && !no_wildcard(arg))
 			recover_with_dwim = 0;
 		/*
 		 * Accept "git checkout foo" and "git checkout foo --"
diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index 3e5ac81bd2..ea95fb8668 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -215,4 +215,35 @@ test_expect_success 'loosely defined local base branch is reported correctly' '
 	test_cmp expect actual
 '
 
+test_expect_success 'reject when arg could be part of dwim branch' '
+	git remote add foo file://non-existent-place &&
+	git update-ref refs/remotes/foo/dwim-arg HEAD &&
+	echo foo >dwim-arg &&
+	git add dwim-arg &&
+	echo bar >dwim-arg &&
+	test_must_fail git checkout dwim-arg &&
+	test_must_fail git rev-parse refs/heads/dwim-arg -- &&
+	grep bar dwim-arg
+'
+
+test_expect_success 'disambiguate dwim branch and checkout path (1)' '
+	git update-ref refs/remotes/foo/dwim-arg1 HEAD &&
+	echo foo >dwim-arg1 &&
+	git add dwim-arg1 &&
+	echo bar >dwim-arg1 &&
+	git checkout -- dwim-arg1 &&
+	test_must_fail git rev-parse refs/heads/dwim-arg1 -- &&
+	grep foo dwim-arg1
+'
+
+test_expect_success 'disambiguate dwim branch and checkout path (2)' '
+	git update-ref refs/remotes/foo/dwim-arg2 HEAD &&
+	echo foo >dwim-arg2 &&
+	git add dwim-arg2 &&
+	echo bar >dwim-arg2 &&
+	git checkout dwim-arg2 -- &&
+	git rev-parse refs/heads/dwim-arg2 -- &&
+	grep bar dwim-arg2
+'
+
 test_done
-- 8< --