{"thread":{"id":"39753","subject":"[PATCH] Avoid the need of \"--\" when wildcard pathspec is used","startedAt":"2015-06-30T12:40:07Z","lastAt":"2015-07-01T16:31:30Z","messageCount":5,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"265224","messageId":"1435668007-31231-1-git-send-email-pclouds@gmail.com","threadId":"39753","inReplyTo":null,"subject":"[PATCH] Avoid the need of \"--\" when wildcard pathspec is used","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2015-06-30T12:40:07Z","receivedAt":"2015-06-30T12:40:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When \"--\" is lacking from the command line and a command can take both\nrevs and paths, the idea is if an argument can be seen as both an\nextended SHA-1 and a path, then \"--\" is required or git refuses to\ncontinue. It's currently implemented as:\n\n(1) if an argument is rev, then it must not exist in worktree\n\n(2) else, it must exist in worktree\n\n(3) else, \"--\" is required.\n\nThese rules work for literal paths, but when wildcard pathspec is\ninvolved, it always requires the user to add \"--\" because it fails (2)\nand (1) is never met.\n\nThis patch prefers wildcard over extended sha-1 syntax that includes\nwildcards, so that we can specify wildcards to select paths in worktree\nwithout \"--\" most of the time. If wildcards are found in extended sha-1\nsyntax, then \"--\" is _always_ required.\n\nBecause ref names don't allow wildcards, you can only hit that\n\"needs '--'\" new rule if you use \":/<pattern>\", \"<rev>^{/<pattern>}\" or\n\"<rev>:wildcards/in/literal/paths\". So it's really rare.\n\nThe rules after this patch become:\n\n(1) if an arg is a rev, then it must either exist in worktree or not\n    a wild card\n\n(2) else, it either exists in worktree or is a wild card\n\n(3) else, \"--\" is required.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n setup.c                           |  2 ++\n t/t2019-checkout-ambiguous-ref.sh | 26 ++++++++++++++++++++++++++\n 2 files changed, 28 insertions(+)\n\ndiff --git a/setup.c b/setup.c\nindex 82c0cc2..f7cb93b 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -143,6 +143,8 @@ int check_filename(const char *prefix, const char *arg)\n \t\tname = arg + 2;\n \t} else if (!no_wildcard(arg))\n \t\treturn 1;\n+\telse if (!no_wildcard(arg))\n+\t\treturn 1;\n \telse if (prefix)\n \t\tname = prefix_filename(prefix, strlen(prefix), arg);\n \telse\ndiff --git a/t/t2019-checkout-ambiguous-ref.sh b/t/t2019-checkout-ambiguous-ref.sh\nindex b99d519..e5ceba3 100755\n--- a/t/t2019-checkout-ambiguous-ref.sh\n+++ b/t/t2019-checkout-ambiguous-ref.sh\n@@ -56,4 +56,30 @@ test_expect_success VAGUENESS_SUCCESS 'checkout reports switch to branch' '\n \ttest_i18ngrep ! \"^HEAD is now at\" stderr\n '\n \n+test_expect_success 'wildcard ambiguation' '\n+\tgit init ambi &&\n+\t(\n+\t\tcd ambi &&\n+\t\techo a >a.c &&\n+\t\tgit add a.c &&\n+\t\techo b >a.c &&\n+\t\tgit checkout \"*.c\" &&\n+\t\techo a >expect &&\n+\t\ttest_cmp expect a.c\n+\t)\n+'\n+\n+test_expect_success 'wildcard ambiguation (2)' '\n+\tgit init ambi2 &&\n+\t(\n+\t\tcd ambi2 &&\n+\t\techo a >\"*.c\" &&\n+\t\tgit add . &&\n+\t\ttest_must_fail git show :\"*.c\" &&\n+\t\tgit show :\"*.c\" -- >actual &&\n+\t\techo a >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \n2.3.0.rc1.137.g477eb31\n"},{"id":"265261","messageId":"xmqqk2ulcbj3.fsf@gitster.dls.corp.google.com","threadId":"39753","inReplyTo":"1435668007-31231-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] Avoid the need of \"--\" when wildcard pathspec is used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-30T18:10:40Z","receivedAt":"2015-06-30T18:10:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> When \"--\" is lacking from the command line and a command can take both\n> revs and paths, the idea is if an argument can be seen as both an\n> extended SHA-1 and a path, then \"--\" is required or git refuses to\n> continue. It's currently implemented as:\n> ...\n\nHmph, how does this relate to 28fcc0b7 (pathspec: avoid the need of\n\"--\" when wildcard is used, 2015-05-02)?  A follow-up?  \"Oops, I did\nit wrong\"?  something else?\n\n> diff --git a/setup.c b/setup.c\n> index 82c0cc2..f7cb93b 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -143,6 +143,8 @@ int check_filename(const char *prefix, const char *arg)\n>  \t\tname = arg + 2;\n>  \t} else if (!no_wildcard(arg))\n>  \t\treturn 1;\n> +\telse if (!no_wildcard(arg))\n> +\t\treturn 1;\n\nPuzzling.  You already checked if arg has an wildcard and returned\nwith 1 if there is none.  The added check looks like a no-op to me.\n\n> diff --git a/t/t2019-checkout-ambiguous-ref.sh b/t/t2019-checkout-ambiguous-ref.sh\n> index b99d519..e5ceba3 100755\n> --- a/t/t2019-checkout-ambiguous-ref.sh\n> +++ b/t/t2019-checkout-ambiguous-ref.sh\n> @@ -56,4 +56,30 @@ test_expect_success VAGUENESS_SUCCESS 'checkout reports switch to branch' '\n>  \ttest_i18ngrep ! \"^HEAD is now at\" stderr\n>  '\n>  \n> +test_expect_success 'wildcard ambiguation' '\n> +\tgit init ambi &&\n> +\t(\n> +\t\tcd ambi &&\n> +\t\techo a >a.c &&\n> +\t\tgit add a.c &&\n> +\t\techo b >a.c &&\n> +\t\tgit checkout \"*.c\" &&\n> +\t\techo a >expect &&\n> +\t\ttest_cmp expect a.c\n> +\t)\n> +'\n> +\n> +test_expect_success 'wildcard ambiguation (2)' '\n> +\tgit init ambi2 &&\n> +\t(\n> +\t\tcd ambi2 &&\n> +\t\techo a >\"*.c\" &&\n> +\t\tgit add . &&\n> +\t\ttest_must_fail git show :\"*.c\" &&\n> +\t\tgit show :\"*.c\" -- >actual &&\n> +\t\techo a >expect &&\n> +\t\ttest_cmp expect actual\n> +\t)\n> +'\n> +\n>  test_done\n"},{"id":"265281","messageId":"CACsJy8C0Pz8ZjzW+n=3kT3c+x+CBONUuk27SNMiMsYwCapnVjA@mail.gmail.com","threadId":"39753","inReplyTo":"xmqqk2ulcbj3.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Avoid the need of \"--\" when wildcard pathspec is used","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-06-30T23:07:27Z","receivedAt":"2015-06-30T23:07:27Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Jul 1, 2015 at 1:10 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> When \"--\" is lacking from the command line and a command can take both\n>> revs and paths, the idea is if an argument can be seen as both an\n>> extended SHA-1 and a path, then \"--\" is required or git refuses to\n>> continue. It's currently implemented as:\n>> ...\n>\n> Hmph, how does this relate to 28fcc0b7 (pathspec: avoid the need of\n> \"--\" when wildcard is used, 2015-05-02)?  A follow-up?  \"Oops, I did\n> it wrong\"?  something else?\n\nArghhh! I vaguely recalled I sent something but I couldn't find it and..\n\n>\n>> diff --git a/setup.c b/setup.c\n>> index 82c0cc2..f7cb93b 100644\n>> --- a/setup.c\n>> +++ b/setup.c\n>> @@ -143,6 +143,8 @@ int check_filename(const char *prefix, const char *arg)\n>>               name = arg + 2;\n>>       } else if (!no_wildcard(arg))\n>>               return 1;\n\n.. if I looked at the context lines, I should have noticed the change\nwas already here!\n\n>> +     else if (!no_wildcard(arg))\n>> +             return 1;\n\n Seems strange (or expected?) that git cherry-pick just adds this\nchunk on top anyway..\n\n> Puzzling.  You already checked if arg has an wildcard and returned\n> with 1 if there is none.  The added check looks like a no-op to me.\n\nYeah sorry for the noise. The only value this patch adds is tests (and\nmaybe better commit message, the last one still mentions magic\npathspec even though it's not about it). I think we can just drop\nthis.\n-- \nDuy\n"},{"id":"265319","messageId":"1435748894-4636-1-git-send-email-pclouds@gmail.com","threadId":"39753","inReplyTo":"1435668007-31231-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2] Add tests for wildcard \"path vs ref\" disambiguation","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2015-07-01T11:08:14Z","receivedAt":"2015-07-01T11:08:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Commit 28fcc0b (pathspec: avoid the need of \"--\" when wildcard is used -\n2015-05-02) changes how the disambiguation rules work. This patch adds\nsome tests to demonstrate, basically, if wildcard characters are in an\nargument:\n\n - if the argument is valid extended sha-1 syntax, \"--\" must be used\n - otherwise the argument is considered a path, even without \"--\"\n\nAnd wildcard can appear in extended sha-1 syntax, either as part of\nregex in \":/<regex>\" or as the literal path in \":<path>\". The latter\ncase is less likely to happen in real world. But if you do \":/\" a lot,\nyou may need to type \"--\" more.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n v2 just adds tests as the code is already in 'master' for some time.\n\n t/t2019-checkout-ambiguous-ref.sh | 26 ++++++++++++++++++++++++++\n 1 file changed, 26 insertions(+)\n\ndiff --git a/t/t2019-checkout-ambiguous-ref.sh b/t/t2019-checkout-ambiguous-ref.sh\nindex b99d519..e5ceba3 100755\n--- a/t/t2019-checkout-ambiguous-ref.sh\n+++ b/t/t2019-checkout-ambiguous-ref.sh\n@@ -56,4 +56,30 @@ test_expect_success VAGUENESS_SUCCESS 'checkout reports switch to branch' '\n \ttest_i18ngrep ! \"^HEAD is now at\" stderr\n '\n \n+test_expect_success 'wildcard ambiguation, paths win' '\n+\tgit init ambi &&\n+\t(\n+\t\tcd ambi &&\n+\t\techo a >a.c &&\n+\t\tgit add a.c &&\n+\t\techo b >a.c &&\n+\t\tgit checkout \"*.c\" &&\n+\t\techo a >expect &&\n+\t\ttest_cmp expect a.c\n+\t)\n+'\n+\n+test_expect_success 'wildcard ambiguation, refs lose' '\n+\tgit init ambi2 &&\n+\t(\n+\t\tcd ambi2 &&\n+\t\techo a >\"*.c\" &&\n+\t\tgit add . &&\n+\t\ttest_must_fail git show :\"*.c\" &&\n+\t\tgit show :\"*.c\" -- >actual &&\n+\t\techo a >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \n2.3.0.rc1.137.g477eb31\n"},{"id":"265326","messageId":"xmqqzj3falgd.fsf@gitster.dls.corp.google.com","threadId":"39753","inReplyTo":"1435748894-4636-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2] Add tests for wildcard \"path vs ref\" disambiguation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T16:31:30Z","receivedAt":"2015-07-01T16:31:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, will queue.\n"}]}