[PATCH] Avoid the need of "--" when wildcard pathspec is used

[PATCH] Avoid the need of "--" when wildcard pathspec is used

[Nguyễn Thái Ngọc Duy]

When "--" is lacking from the command line and a command can take both
revs and paths, the idea is if an argument can be seen as both an
extended SHA-1 and a path, then "--" is required or git refuses to
continue. It's currently implemented as:

(1) if an argument is rev, then it must not exist in worktree

(2) else, it must exist in worktree

(3) else, "--" is required.

These rules work for literal paths, but when wildcard pathspec is
involved, it always requires the user to add "--" because it fails (2)
and (1) is never met.

This patch prefers wildcard over extended sha-1 syntax that includes
wildcards, so that we can specify wildcards to select paths in worktree
without "--" most of the time. If wildcards are found in extended sha-1
syntax, then "--" is _always_ required.

Because ref names don't allow wildcards, you can only hit that
"needs '--'" new rule if you use ":/<pattern>", "<rev>^{/<pattern>}" or
"<rev>:wildcards/in/literal/paths". So it's really rare.

The rules after this patch become:

(1) if an arg is a rev, then it must either exist in worktree or not
    a wild card

(2) else, it either exists in worktree or is a wild card

(3) else, "--" is required.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 setup.c                           |  2 ++
 t/t2019-checkout-ambiguous-ref.sh | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/setup.c b/setup.c
index 82c0cc2..f7cb93b 100644
--- a/setup.c
+++ b/setup.c
@@ -143,6 +143,8 @@ int check_filename(const char *prefix, const char *arg)
 		name = arg + 2;
 	} else if (!no_wildcard(arg))
 		return 1;
+	else if (!no_wildcard(arg))
+		return 1;
 	else if (prefix)
 		name = prefix_filename(prefix, strlen(prefix), arg);
 	else
diff --git a/t/t2019-checkout-ambiguous-ref.sh b/t/t2019-checkout-ambiguous-ref.sh
index b99d519..e5ceba3 100755
--- a/t/t2019-checkout-ambiguous-ref.sh
+++ b/t/t2019-checkout-ambiguous-ref.sh
@@ -56,4 +56,30 @@ test_expect_success VAGUENESS_SUCCESS 'checkout reports switch to branch' '
 	test_i18ngrep ! "^HEAD is now at" stderr
 '
 
+test_expect_success 'wildcard ambiguation' '
+	git init ambi &&
+	(
+		cd ambi &&
+		echo a >a.c &&
+		git add a.c &&
+		echo b >a.c &&
+		git checkout "*.c" &&
+		echo a >expect &&
+		test_cmp expect a.c
+	)
+'
+
+test_expect_success 'wildcard ambiguation (2)' '
+	git init ambi2 &&
+	(
+		cd ambi2 &&
+		echo a >"*.c" &&
+		git add . &&
+		test_must_fail git show :"*.c" &&
+		git show :"*.c" -- >actual &&
+		echo a >expect &&
+		test_cmp expect actual
+	)
+'
+
 test_done
-- 
2.3.0.rc1.137.g477eb31

Re: [PATCH] Avoid the need of "--" when wildcard pathspec is used

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> When "--" is lacking from the command line and a command can take both
> revs and paths, the idea is if an argument can be seen as both an
> extended SHA-1 and a path, then "--" is required or git refuses to
> continue. It's currently implemented as:
> ...

Hmph, how does this relate to 28fcc0b7 (pathspec: avoid the need of
"--" when wildcard is used, 2015-05-02)?  A follow-up?  "Oops, I did
it wrong"?  something else?

> diff --git a/setup.c b/setup.c
> index 82c0cc2..f7cb93b 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -143,6 +143,8 @@ int check_filename(const char *prefix, const char *arg)
>  		name = arg + 2;
>  	} else if (!no_wildcard(arg))
>  		return 1;
> +	else if (!no_wildcard(arg))
> +		return 1;

Puzzling.  You already checked if arg has an wildcard and returned
with 1 if there is none.  The added check looks like a no-op to me.

> diff --git a/t/t2019-checkout-ambiguous-ref.sh b/t/t2019-checkout-ambiguous-ref.sh
> index b99d519..e5ceba3 100755
> --- a/t/t2019-checkout-ambiguous-ref.sh
> +++ b/t/t2019-checkout-ambiguous-ref.sh
> @@ -56,4 +56,30 @@ test_expect_success VAGUENESS_SUCCESS 'checkout reports switch to branch' '
>  	test_i18ngrep ! "^HEAD is now at" stderr
>  '
>  
> +test_expect_success 'wildcard ambiguation' '
> +	git init ambi &&
> +	(
> +		cd ambi &&
> +		echo a >a.c &&
> +		git add a.c &&
> +		echo b >a.c &&
> +		git checkout "*.c" &&
> +		echo a >expect &&
> +		test_cmp expect a.c
> +	)
> +'
> +
> +test_expect_success 'wildcard ambiguation (2)' '
> +	git init ambi2 &&
> +	(
> +		cd ambi2 &&
> +		echo a >"*.c" &&
> +		git add . &&
> +		test_must_fail git show :"*.c" &&
> +		git show :"*.c" -- >actual &&
> +		echo a >expect &&
> +		test_cmp expect actual
> +	)
> +'
> +
>  test_done

Re: [PATCH] Avoid the need of "--" when wildcard pathspec is used

[Duy Nguyen]

On Wed, Jul 1, 2015 at 1:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> When "--" is lacking from the command line and a command can take both
>> revs and paths, the idea is if an argument can be seen as both an
>> extended SHA-1 and a path, then "--" is required or git refuses to
>> continue. It's currently implemented as:
>> ...
>
> Hmph, how does this relate to 28fcc0b7 (pathspec: avoid the need of
> "--" when wildcard is used, 2015-05-02)?  A follow-up?  "Oops, I did
> it wrong"?  something else?

Arghhh! I vaguely recalled I sent something but I couldn't find it and..

>
>> diff --git a/setup.c b/setup.c
>> index 82c0cc2..f7cb93b 100644
>> --- a/setup.c
>> +++ b/setup.c
>> @@ -143,6 +143,8 @@ int check_filename(const char *prefix, const char *arg)
>>               name = arg + 2;
>>       } else if (!no_wildcard(arg))
>>               return 1;

.. if I looked at the context lines, I should have noticed the change
was already here!

>> +     else if (!no_wildcard(arg))
>> +             return 1;

 Seems strange (or expected?) that git cherry-pick just adds this
chunk on top anyway..

> Puzzling.  You already checked if arg has an wildcard and returned
> with 1 if there is none.  The added check looks like a no-op to me.

Yeah sorry for the noise. The only value this patch adds is tests (and
maybe better commit message, the last one still mentions magic
pathspec even though it's not about it). I think we can just drop
this.
-- 
Duy

[PATCH v2] Add tests for wildcard "path vs ref" disambiguation

[Nguyễn Thái Ngọc Duy]

Commit 28fcc0b (pathspec: avoid the need of "--" when wildcard is used -
2015-05-02) changes how the disambiguation rules work. This patch adds
some tests to demonstrate, basically, if wildcard characters are in an
argument:

 - if the argument is valid extended sha-1 syntax, "--" must be used
 - otherwise the argument is considered a path, even without "--"

And wildcard can appear in extended sha-1 syntax, either as part of
regex in ":/<regex>" or as the literal path in ":<path>". The latter
case is less likely to happen in real world. But if you do ":/" a lot,
you may need to type "--" more.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 v2 just adds tests as the code is already in 'master' for some time.

 t/t2019-checkout-ambiguous-ref.sh | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/t/t2019-checkout-ambiguous-ref.sh b/t/t2019-checkout-ambiguous-ref.sh
index b99d519..e5ceba3 100755
--- a/t/t2019-checkout-ambiguous-ref.sh
+++ b/t/t2019-checkout-ambiguous-ref.sh
@@ -56,4 +56,30 @@ test_expect_success VAGUENESS_SUCCESS 'checkout reports switch to branch' '
 	test_i18ngrep ! "^HEAD is now at" stderr
 '
 
+test_expect_success 'wildcard ambiguation, paths win' '
+	git init ambi &&
+	(
+		cd ambi &&
+		echo a >a.c &&
+		git add a.c &&
+		echo b >a.c &&
+		git checkout "*.c" &&
+		echo a >expect &&
+		test_cmp expect a.c
+	)
+'
+
+test_expect_success 'wildcard ambiguation, refs lose' '
+	git init ambi2 &&
+	(
+		cd ambi2 &&
+		echo a >"*.c" &&
+		git add . &&
+		test_must_fail git show :"*.c" &&
+		git show :"*.c" -- >actual &&
+		echo a >expect &&
+		test_cmp expect actual
+	)
+'
+
 test_done
-- 
2.3.0.rc1.137.g477eb31

Re: [PATCH v2] Add tests for wildcard "path vs ref" disambiguation

[Junio C Hamano]

Thanks, will queue.