{"thread":{"id":"43903","subject":"[PATCH] checkout: swap the order of ambiguity check for :/ syntax","startedAt":"2016-08-22T12:36:35Z","lastAt":"2016-09-09T19:21:12Z","messageCount":12,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"299837","messageId":"20160822123502.3521-1-pclouds@gmail.com","threadId":"43903","inReplyTo":null,"subject":"[PATCH] checkout: swap the order of ambiguity check for :/ syntax","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-08-22T12:35:02Z","receivedAt":"2016-08-22T12:36:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This should speed up \"git checkout :/long/path/taken/from/diffstat\"\nbecause we don't have to walk through the commit graph to see if\n\"long/path/taken/from/diffstat\" exists in any commit message.\n\nThis is in the the same spirit as 4db86e8 (Update :/abc ambiguity check\n- 2013-01-21), but instead of considering this case (\"abc\" in \":/abc\"\nexists on worktree) ambiguous and shouting, we just assume the user\nmeans \"pathspec\" not \"ref\".\n\nIt's not wonderful, but it's in line with how git-checkout stops caring\nabout ambiguity after the first argument can be resolved as a ref\n(there's even a test for it, t2010.6). In other words, we assume the\nuser means \"ref\" not \"pathspec\" when that happens. Here we simply swap\nthe order of checking specifically for :/ on practical ground.\n\nLast note, to be pedantic, we should check if \"abc\" from \":/abc\" exists\nas an _index_ entry, not on disk. But chances are they exist in both\nplaces anyway...\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/checkout.c | 16 ++++++++++++++++\n 1 file changed, 16 insertions(+)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 8672d07..6f016db 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -973,6 +973,22 @@ static int parse_branchname_arg(int argc, const char **argv,\n \tif (!strcmp(arg, \"-\"))\n \t\targ = \"@{-1}\";\n \n+\tif (dash_dash_pos < 0 && starts_with(arg, \":/\") &&\n+\t check_filename(opts->prefix, arg)) {\n+\t\t/*\n+\t\t * Normally if the first argument is ambiguous, we\n+\t\t * choose to believe the user specifies an extended\n+\t\t * SHA-1 syntax unless it turns out not true, then we\n+\t\t * see if it's a pathspec.\n+\t\t *\n+\t\t * :/ here is an exception because resolving :/abc may\n+\t\t * involve walking through the entire commit\n+\t\t * graph. Expensive and slow. If :/abc points to an\n+\t\t * existing file, ignore ambiguity and go with\n+\t\t * pathspec (i.e. skip get_sha1_mb()).\n+\t\t */\n+\t\treturn 0;\t/* case (2) */\n+\t}\n \tif (get_sha1_mb(arg, rev)) {\n \t\t/*\n \t\t * Either case (3) or (4), with not being\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"300077","messageId":"xmqq7fb6rupa.fsf@gitster.mtv.corp.google.com","threadId":"43903","inReplyTo":"20160822123502.3521-1-pclouds@gmail.com","subject":"Re: [PATCH] checkout: swap the order of ambiguity check for :/ syntax","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-24T16:35:29Z","receivedAt":"2016-08-24T16:41:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> It's not wonderful, but it's in line with how git-checkout stops caring\n> about ambiguity after the first argument can be resolved as a ref\n> (there's even a test for it, t2010.6).\n\nBut that is justifiable because checkout can only ever take one\nrevision. What follows, if there are any, must be paths, and more\nimportantly, it would be perfectly reasonable if some of them were\nmissing in the working tree (\"ow, I accidentally removed that file,\nI need to resurrect it from the index\"). Does the same justification\napply to this change?\n\n"},{"id":"300136","messageId":"CACsJy8AnDoh-CZp8oa21YHcM3sxoTJZiK7fP+ecNyqd0-DBZAA@mail.gmail.com","threadId":"43903","inReplyTo":"xmqq7fb6rupa.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] checkout: swap the order of ambiguity check for :/ syntax","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-08-25T09:21:25Z","receivedAt":"2016-08-25T09:22:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Aug 24, 2016 at 11:35 PM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> It's not wonderful, but it's in line with how git-checkout stops caring\n>> about ambiguity after the first argument can be resolved as a ref\n>> (there's even a test for it, t2010.6).\n>\n> But that is justifiable because checkout can only ever take one\n> revision. What follows, if there are any, must be paths, and more\n> importantly, it would be perfectly reasonable if some of them were\n> missing in the working tree (\"ow, I accidentally removed that file,\n> I need to resurrect it from the index\"). Does the same justification\n> apply to this change?\n\nI think there is a misunderstanding. My \"after\" is in \"after the first\nargument can be resolved, check if it exists in worktree too, if so\nit's ambiguous and bail\". This is usually how we detect ambiguation.\nBut git-checkout does not do the \"check if it exists...\" clause.\n-- \nDuy\n"},{"id":"300175","messageId":"xmqq60qoojf7.fsf@gitster.mtv.corp.google.com","threadId":"43903","inReplyTo":"CACsJy8AnDoh-CZp8oa21YHcM3sxoTJZiK7fP+ecNyqd0-DBZAA@mail.gmail.com","subject":"Re: [PATCH] checkout: swap the order of ambiguity check for :/ syntax","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-25T17:19:40Z","receivedAt":"2016-08-25T17:25:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Wed, Aug 24, 2016 at 11:35 PM, Junio C Hamano wrote:\n>> Nguyễn Thái Ngọc Duy writes:\n>>\n>>> It's not wonderful, but it's in line with how git-checkout stops caring\n>>> about ambiguity after the first argument can be resolved as a ref\n>>> (there's even a test for it, t2010.6).\n>>\n>> But that is justifiable because checkout can only ever take one\n>> revision. What follows, if there are any, must be paths, and more\n>> importantly, it would be perfectly reasonable if some of them were\n>> missing in the working tree (\"ow, I accidentally removed that file,\n>> I need to resurrect it from the index\"). Does the same justification\n>> apply to this change?\n>\n> I think there is a misunderstanding. My \"after\" is in \"after the first\n> argument can be resolved, check if it exists in worktree too, if so\n> it's ambiguous and bail\". This is usually how we detect ambiguation.\n> But git-checkout does not do the \"check if it exists...\" clause.\n\nHmph. The \"case 4\" in the function you touched says\n\n * case 4: git checkout \n *\n * The first argument must not be ambiguous.\n * - If it's *only* a reference, treat it like case (1).\n * - If it's only a path, treat it like case (2).\n * - else: fail.\n\nDid we break it recently?\n"},{"id":"300238","messageId":"CACsJy8CKH35T-Vtdsds6Q8RUbO-UxJ7CROeLGjf_-QaXMw-awQ@mail.gmail.com","threadId":"43903","inReplyTo":"xmqq60qoojf7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] checkout: swap the order of ambiguity check for :/ syntax","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-08-26T13:34:27Z","receivedAt":"2016-08-26T13:41:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Aug 26, 2016 at 12:19 AM, Junio C Hamano wrote:\n> Duy Nguyen writes:\n>\n>> On Wed, Aug 24, 2016 at 11:35 PM, Junio C Hamano wrote:\n>>> Nguyễn Thái Ngọc Duy writes:\n>>>\n>>>> It's not wonderful, but it's in line with how git-checkout stops caring\n>>>> about ambiguity after the first argument can be resolved as a ref\n>>>> (there's even a test for it, t2010.6).\n>>>\n>>> But that is justifiable because checkout can only ever take one\n>>> revision. What follows, if there are any, must be paths, and more\n>>> importantly, it would be perfectly reasonable if some of them were\n>>> missing in the working tree (\"ow, I accidentally removed that file,\n>>> I need to resurrect it from the index\"). Does the same justification\n>>> apply to this change?\n>>\n>> I think there is a misunderstanding. My \"after\" is in \"after the first\n>> argument can be resolved, check if it exists in worktree too, if so\n>> it's ambiguous and bail\". This is usually how we detect ambiguation.\n>> But git-checkout does not do the \"check if it exists...\" clause.\n>\n> Hmph. The \"case 4\" in the function you touched says\n>\n> * case 4: git checkout \n> *\n> * The first argument must not be ambiguous.\n> * - If it's *only* a reference, treat it like case (1).\n> * - If it's only a path, treat it like case (2).\n> * - else: fail.\n>\n> Did we break it recently?\n\nI was driven by testing and did not read the code carefully (but on\nthe other hand, ambiguation plus dwim is never a pleasant read). Near\nthe end of this function we have\n\n if (!has_dash_dash) {/* case (3).(d) -> (1) */\n /*\n * Do not complain the most common case\n * git checkout branch\n * even if there happen to be a file called 'branch';\n * it would be extremely annoying.\n */\n if (argc)\n verify_non_filename(NULL, arg);\n\nargc == 0 would be case 3d, argc != 0 is case 4 and \"touch abc; git\ncheckout :/abc def\" does complain about :/abc being ambiguous. So no\nwe haven't broken anything yet, but I do with my patch. If I want to\nmake a special case for :/ to speed it up, I need to be careful about\nthis (and I probably need a test too because it's not detected).\n\nWhile at there, verify_non_filename() in this function (and\ncheck_filename() too) should pass the right prefix in argument 1, not\nNULL, I think. I'll need to do some digging to see why they are NULL\nin the first place though.\n-- \nDuy\n"},{"id":"301252","messageId":"20160907111941.2342-1-pclouds@gmail.com","threadId":"43903","inReplyTo":"20160822123502.3521-1-pclouds@gmail.com","subject":"[PATCH 0/3] fix checkout ambiguation in subdir","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-07T11:19:38Z","receivedAt":"2016-09-07T11:21:10Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"I need some more time (which I don't have) to convince myself about\nthe \"git checkout :/abc\" patch. But these look like good bug\nfix/improvement.\n\nNguyễn Thái Ngọc Duy (3):\n checkout: add some spaces between code and comment\n checkout.txt: document a common case that ignores ambiguation rules\n checkout: fix ambiguity check in subdir\n\n Documentation/git-checkout.txt | 9 +++++++++\n builtin/checkout.c | 6 +++---\n t/t2010-checkout-ambiguous.sh | 9 +++++++++\n t/t2024-checkout-dwim.sh | 12 ++++++++++++\n 4 files changed, 33 insertions(+), 3 deletions(-)\n\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"301253","messageId":"20160907111941.2342-2-pclouds@gmail.com","threadId":"43903","inReplyTo":"20160907111941.2342-1-pclouds@gmail.com","subject":"[PATCH 1/3] checkout: add some spaces between code and comment","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-07T11:19:39Z","receivedAt":"2016-09-07T11:21:13Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/checkout.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 8672d07..1f71d06 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1038,7 +1038,7 @@ static int parse_branchname_arg(int argc, const char **argv,\n \n \tif (!*source_tree) /* case (1): want a tree */\n \t\tdie(_(\"reference is not a tree: %s\"), arg);\n-\tif (!has_dash_dash) {/* case (3).(d) -> (1) */\n+\tif (!has_dash_dash) {\t/* case (3).(d) -> (1) */\n \t\t/*\n \t\t * Do not complain the most common case\n \t\t *\tgit checkout branch\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"301254","messageId":"20160907111941.2342-3-pclouds@gmail.com","threadId":"43903","inReplyTo":"20160907111941.2342-1-pclouds@gmail.com","subject":"[PATCH 2/3] checkout.txt: document a common case that ignores ambiguation rules","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-07T11:19:40Z","receivedAt":"2016-09-07T11:21:21Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Normally we err on the safe side: if something can be seen as both an\nSHA1 and a pathspec, we stop and scream. In checkout, there is one\nexception added in 859fdab (git-checkout: improve error messages, detect\nambiguities. - 2008-07-23), to allow the common case \"git checkout\nbranch\". Let's document this exception.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/git-checkout.txt | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex 7a2201b..94eb238 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -419,6 +419,15 @@ $ git reflog -2 HEAD # or\n $ git log -g -2 HEAD\n ------------\n \n+ARGUMENT AMBIGUATION\n+--------------------\n+\n+When there is only one argument given and it is not `--` (e.g. \"git\n+checkout abc\"), \"abc\" could be seen as either a `` or a\n+``, but Git will assume the argument is a ``, which is\n+a common case for switching branches. Use `git checkout -- `\n+form if you mean it to be a pathspec.\n+\n EXAMPLES\n --------\n \n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"301255","messageId":"20160907111941.2342-4-pclouds@gmail.com","threadId":"43903","inReplyTo":"20160907111941.2342-1-pclouds@gmail.com","subject":"[PATCH 3/3] checkout: fix ambiguity check in subdir","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-07T11:19:41Z","receivedAt":"2016-09-07T11:21:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The two functions in parse_branchname_arg(), verify_non_filename and\ncheck_filename, need correct prefix in order to reconstruct the paths\nand check for their existence. With NULL prefix, they just check paths\nat top dir instead.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/checkout.c | 4 ++--\n t/t2010-checkout-ambiguous.sh | 9 +++++++++\n t/t2024-checkout-dwim.sh | 12 ++++++++++++\n 3 files changed, 23 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 1f71d06..53c7284 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -985,7 +985,7 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\tint recover_with_dwim = dwim_new_local_branch_ok;\n \n \t\tif (!has_dash_dash &&\n-\t\t (check_filename(NULL, arg) || !no_wildcard(arg)))\n+\t\t (check_filename(opts->prefix, arg) || !no_wildcard(arg)))\n \t\t\trecover_with_dwim = 0;\n \t\t/*\n \t\t * Accept \"git checkout foo\" and \"git checkout foo --\"\n@@ -1046,7 +1046,7 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\t * it would be extremely annoying.\n \t\t */\n \t\tif (argc)\n-\t\t\tverify_non_filename(NULL, arg);\n+\t\t\tverify_non_filename(opts->prefix, arg);\n \t} else {\n \t\targcount++;\n \t\targv++;\ndiff --git a/t/t2010-checkout-ambiguous.sh b/t/t2010-checkout-ambiguous.sh\nindex e76e84a..2e47fe0 100755\n--- a/t/t2010-checkout-ambiguous.sh\n+++ b/t/t2010-checkout-ambiguous.sh\n@@ -41,6 +41,15 @@ test_expect_success 'check ambiguity' '\n \ttest_must_fail git checkout world all\n '\n \n+test_expect_success 'check ambiguity in subdir' '\n+\tmkdir sub &&\n+\t# not ambiguous because sub/world does not exist\n+\tgit -C sub checkout world ../all &&\n+\techo hello >sub/world &&\n+\t# ambiguous because sub/world does exist\n+\ttest_must_fail git -C sub checkout world ../all\n+'\n+\n test_expect_success 'disambiguate checking out from a tree-ish' '\n \techo bye > world &&\n \tgit checkout world -- world &&\ndiff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh\nindex 468a000..3e5ac81 100755\n--- a/t/t2024-checkout-dwim.sh\n+++ b/t/t2024-checkout-dwim.sh\n@@ -174,6 +174,18 @@ test_expect_success 'checkout of branch with a file having the same name fails'\n \ttest_branch master\n '\n \n+test_expect_success 'checkout of branch with a file in subdir having the same name fails' '\n+\tgit checkout -B master &&\n+\ttest_might_fail git branch -D spam &&\n+\n+\t>spam &&\n+\tmkdir sub &&\n+\tmv spam sub/spam &&\n+\ttest_must_fail git -C sub checkout spam &&\n+\ttest_must_fail git rev-parse --verify refs/heads/spam &&\n+\ttest_branch master\n+'\n+\n test_expect_success 'checkout -- succeeds, even if a file with the same name exists' '\n \tgit checkout -B master &&\n \ttest_might_fail git branch -D spam &&\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"301439","messageId":"xmqq7famgnwo.fsf@gitster.mtv.corp.google.com","threadId":"43903","inReplyTo":"20160907111941.2342-3-pclouds@gmail.com","subject":"Re: [PATCH 2/3] checkout.txt: document a common case that ignores ambiguation rules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-08T20:03:03Z","receivedAt":"2016-09-08T20:03:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Normally we err on the safe side: if something can be seen as both an\n> SHA1 and a pathspec, we stop and scream. In checkout, there is one\n> exception added in 859fdab (git-checkout: improve error messages, detect\n> ambiguities. - 2008-07-23), to allow the common case \"git checkout\n> branch\". Let's document this exception.\n\nGood idea, but...\n\n> +ARGUMENT AMBIGUATION\n> +--------------------\n> +\n> +When there is only one argument given and it is not `--` (e.g. \"git\n> +checkout abc\"), \"abc\" could be seen as either a `` or a\n> +``, but Git will assume the argument is a ``, which is\n> +a common case for switching branches. Use `git checkout -- `\n> +form if you mean it to be a pathspec.\n\n... this is far from reasonable. I'd read \"but Git will assume the\nargument is a tree-ish\" to mean \"git checkout Makefile\" would\nattempt to checkout the Makefile branch and fail.\n\n When there is only one argument given and it is not `--` (e.g. \"git\n checkout abc\"), and when the argument is both a valid ``\n (e.g. a branch \"abc\" exists) and a valid `` (e.g. a file\n or a directory whose name is \"abc\" exists), Git would usually ask\n you to disambiguate. Because checking out a branch is so common an\n operation, however, \"git checkout abc\" takes \"abc\" as a ``\n in such situation. Use `git checkout -- ` if you want to\n checkout these paths out of the index.\n\nor something like that?\n"},{"id":"301441","messageId":"xmqq37lagnuy.fsf@gitster.mtv.corp.google.com","threadId":"43903","inReplyTo":"20160907111941.2342-4-pclouds@gmail.com","subject":"Re: [PATCH 3/3] checkout: fix ambiguity check in subdir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-08T20:04:05Z","receivedAt":"2016-09-08T20:04:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> The two functions in parse_branchname_arg(), verify_non_filename and\n> check_filename, need correct prefix in order to reconstruct the paths\n> and check for their existence. With NULL prefix, they just check paths\n> at top dir instead.\n\nGood eyes. Will queue.\n\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> builtin/checkout.c | 4 ++--\n> t/t2010-checkout-ambiguous.sh | 9 +++++++++\n> t/t2024-checkout-dwim.sh | 12 ++++++++++++\n> 3 files changed, 23 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 1f71d06..53c7284 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -985,7 +985,7 @@ static int parse_branchname_arg(int argc, const char **argv,\n> \t\tint recover_with_dwim = dwim_new_local_branch_ok;\n> \n> \t\tif (!has_dash_dash &&\n> -\t\t (check_filename(NULL, arg) || !no_wildcard(arg)))\n> +\t\t (check_filename(opts->prefix, arg) || !no_wildcard(arg)))\n> \t\t\trecover_with_dwim = 0;\n> \t\t/*\n> \t\t * Accept \"git checkout foo\" and \"git checkout foo --\"\n> @@ -1046,7 +1046,7 @@ static int parse_branchname_arg(int argc, const char **argv,\n> \t\t * it would be extremely annoying.\n> \t\t */\n> \t\tif (argc)\n> -\t\t\tverify_non_filename(NULL, arg);\n> +\t\t\tverify_non_filename(opts->prefix, arg);\n> \t} else {\n> \t\targcount++;\n> \t\targv++;\n> diff --git a/t/t2010-checkout-ambiguous.sh b/t/t2010-checkout-ambiguous.sh\n> index e76e84a..2e47fe0 100755\n> --- a/t/t2010-checkout-ambiguous.sh\n> +++ b/t/t2010-checkout-ambiguous.sh\n> @@ -41,6 +41,15 @@ test_expect_success 'check ambiguity' '\n> \ttest_must_fail git checkout world all\n> '\n> \n> +test_expect_success 'check ambiguity in subdir' '\n> +\tmkdir sub &&\n> +\t# not ambiguous because sub/world does not exist\n> +\tgit -C sub checkout world ../all &&\n> +\techo hello >sub/world &&\n> +\t# ambiguous because sub/world does exist\n> +\ttest_must_fail git -C sub checkout world ../all\n> +'\n> +\n> test_expect_success 'disambiguate checking out from a tree-ish' '\n> \techo bye > world &&\n> \tgit checkout world -- world &&\n> diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh\n> index 468a000..3e5ac81 100755\n> --- a/t/t2024-checkout-dwim.sh\n> +++ b/t/t2024-checkout-dwim.sh\n> @@ -174,6 +174,18 @@ test_expect_success 'checkout of branch with a file having the same name fails'\n> \ttest_branch master\n> '\n> \n> +test_expect_success 'checkout of branch with a file in subdir having the same name fails' '\n> +\tgit checkout -B master &&\n> +\ttest_might_fail git branch -D spam &&\n> +\n> +\t>spam &&\n> +\tmkdir sub &&\n> +\tmv spam sub/spam &&\n> +\ttest_must_fail git -C sub checkout spam &&\n> +\ttest_must_fail git rev-parse --verify refs/heads/spam &&\n> +\ttest_branch master\n> +'\n> +\n> test_expect_success 'checkout -- succeeds, even if a file with the same name exists' '\n> \tgit checkout -B master &&\n> \ttest_might_fail git branch -D spam &&\n"},{"id":"301544","messageId":"xmqqfup8c21t.fsf@gitster.mtv.corp.google.com","threadId":"43903","inReplyTo":"xmqq7famgnwo.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/3] checkout.txt: document a common case that ignores ambiguation rules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-09T19:21:02Z","receivedAt":"2016-09-09T19:21:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I'll queue the following as \"fixup!\" for now. It reminds me that\nthe title also needs rewording; we do not have rules to make things\nambiguous ;-)\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex 46b04b1..8e2c066 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -419,14 +419,17 @@ $ git reflog -2 HEAD # or\n $ git log -g -2 HEAD\n ------------\n \n-ARGUMENT AMBIGUATION\n---------------------\n+ARGUMENT DISAMBIGUATION\n+-----------------------\n \n When there is only one argument given and it is not `--` (e.g. \"git\n-checkout abc\"), \"abc\" could be seen as either a `` or a\n-``, but Git will assume the argument is a ``, which is\n-a common case for switching branches. Use `git checkout -- `\n-form if you mean it to be a pathspec.\n+checkout abc\"), and when the argument is both a valid ``\n+(e.g. a branch \"abc\" exists) and a valid `` (e.g. a file\n+or a directory whose name is \"abc\" exists), Git would usually ask\n+you to disambiguate. Because checking out a branch is so common an\n+operation, however, \"git checkout abc\" takes \"abc\" as a ``\n+in such a situation. Use `git checkout -- ` if you want\n+to checkout these paths out of the index.\n \n EXAMPLES\n --------\n-- \n2.10.0-339-gc0c747f\n\n"}]}