threads / patch / 39753

patchAvoid the need of "--" when wildcard pathspec is used

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

## tl;dr

5 messages between Jun 30, 2015 and Jul 1, 2015. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Nguyễn Thái Ngọc Duy· Jun 30, 2015, 12:40 UTC · lore

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(+)
Show changes to 2 files +28 −0

setup.c, t/t2019-checkout-ambiguous-ref.sh

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
Junio C Hamano· Jun 30, 2015, 18:10 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 5 quoted lines
> 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?

Show 10 quoted lines
> 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.

Show 35 quoted lines
> 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
Duy Nguyen· Jun 30, 2015, 23:07 UTC · re: Junio C Hamano · lore

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

On Wed, Jul 1, 2015 at 1:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 11 quoted lines
> 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..
Show 9 quoted lines
>
>> 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
Nguyễn Thái Ngọc Duy· Jul 1, 2015, 11:08 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(+)
Show changes to t/t2019-checkout-ambiguous-ref.sh +26 −0
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

← back to recent threads