git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] wildmatch: change behavior of "foo**bar" in WM_PATHNAME mode

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Oct 29, 2018, 13:24 UTC
Message-ID
<87h8h4lwcv.fsf@evledraar.gmail.com>
In-Reply-To
<20181027084823.23382-1-pclouds@gmail.com>
On Sat, Oct 27 2018, Nguyễn Thái Ngọc Duy wrote:
Show 89 quoted lines
> In WM_PATHNAME mode (or FNM_PATHNAME), '*' does not match '/' and '**'
> can but only in three patterns:
>
> - '**/' matches zero or more leading directories
> - '/**/' matches zero or more directories in between
> - '/**' matches zero or more trailing directories/files
>
> When '**' is present but not in one of these patterns, the current
> behavior is consider the pattern invalid and stop matching. In other
> words, 'foo**bar' never matches anything, whatever you throw at it.
>
> This behavior is arguably a bit confusing partly because we can't
> really tell the user their pattern is invalid so that they can fix
> it. So instead, tolerate it and make '**' act like two regular '*'s
> (which is essentially the same as a single asterisk). This behavior
> seems more predictable.
>
> Noticed-by: dana <dana@dana.is>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Documentation/gitignore.txt | 3 ++-
>  t/t3070-wildmatch.sh        | 4 ++--
>  wildmatch.c                 | 4 ++--
>  wildmatch.h                 | 1 -
>  4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
> index d107daaffd..1c94f08ff4 100644
> --- a/Documentation/gitignore.txt
> +++ b/Documentation/gitignore.txt
> @@ -129,7 +129,8 @@ full pathname may have special meaning:
>     matches zero or more directories. For example, "`a/**/b`"
>     matches "`a/b`", "`a/x/b`", "`a/x/y/b`" and so on.
>
> - - Other consecutive asterisks are considered invalid.
> + - Other consecutive asterisks are considered regular asterisks and
> +   will match according to the previous rules.
>
>  NOTES
>  -----
> diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh
> index 46aca0af10..891d4d7cb9 100755
> --- a/t/t3070-wildmatch.sh
> +++ b/t/t3070-wildmatch.sh
> @@ -237,7 +237,7 @@ match 0 0 0 0 foobar 'foo\*bar'
>  match 1 1 1 1 'f\oo' 'f\\oo'
>  match 1 1 1 1 ball '*[al]?'
>  match 0 0 0 0 ten '[ten]'
> -match 0 0 1 1 ten '**[!te]'
> +match 1 1 1 1 ten '**[!te]'
>  match 0 0 0 0 ten '**[!ten]'
>  match 1 1 1 1 ten 't[a-g]n'
>  match 0 0 0 0 ten 't[!a-g]n'
> @@ -253,7 +253,7 @@ match 1 1 1 1 ']' ']'
>  # Extended slash-matching features
>  match 0 0 1 1 'foo/baz/bar' 'foo*bar'
>  match 0 0 1 1 'foo/baz/bar' 'foo**bar'
> -match 0 0 1 1 'foobazbar' 'foo**bar'
> +match 1 1 1 1 'foobazbar' 'foo**bar'
>  match 1 1 1 1 'foo/baz/bar' 'foo/**/bar'
>  match 1 1 0 0 'foo/baz/bar' 'foo/**/**/bar'
>  match 1 1 1 1 'foo/b/a/z/bar' 'foo/**/bar'
> diff --git a/wildmatch.c b/wildmatch.c
> index d074c1be10..9e9e2a2f95 100644
> --- a/wildmatch.c
> +++ b/wildmatch.c
> @@ -104,8 +104,8 @@ static int dowild(const uchar *p, const uchar *text, unsigned int flags)
>  					    dowild(p + 1, text, flags) == WM_MATCH)
>  						return WM_MATCH;
>  					match_slash = 1;
> -				} else
> -					return WM_ABORT_MALFORMED;
> +				} else /* WM_PATHNAME is set */
> +					match_slash = 0;
>  			} else
>  				/* without WM_PATHNAME, '*' == '**' */
>  				match_slash = flags & WM_PATHNAME ? 0 : 1;
> diff --git a/wildmatch.h b/wildmatch.h
> index b8c826aa68..5993696298 100644
> --- a/wildmatch.h
> +++ b/wildmatch.h
> @@ -4,7 +4,6 @@
>  #define WM_CASEFOLD 1
>  #define WM_PATHNAME 2
>
> -#define WM_ABORT_MALFORMED 2
>  #define WM_NOMATCH 1
>  #define WM_MATCH 0
>  #define WM_ABORT_ALL -1

This patch looks good to me, but I think it's a bad state of affairs to keep changing these semantics and not having something like a "gitwildmatch" doc were we document this matching syntax.

Also I still need to dig up the work for using PCRE as an alternate matching engine, the PCRE devs produced a bug-for-bug compatible version of our wildmatch function (all the more reason to document it), so I think they'll need to change it now that this is in, but I haven't rebased those ancient patches yet.

Do you have any thoughts on how to proceed with getting this documented / into some stable state where we can specify it? Even if we don't end up using PCRE as a matching engine (sometimes it was faster, sometimes slower) I think it would be very useful if we can spew out "here's your pattern as a regex" for self-documentation purposes.

Then that can be piped into e.g. "perl -Mre=debug" to see a step-by-step guide for how the pattern compiles, and why it does or doesn't match a given thing.

Previous: Junio C HamanoNext: Duy Nguyen
Message 16 of 17 in “[BUG] gitignore documentation inconsistent with actual behaviour”
  1. danaOct 11, 2018
  2. danaOct 11, 2018
  3. Ævar Arnfjörð BjarmasonOct 11, 2018
  4. danaOct 14, 2018
  5. Duy NguyenOct 14, 2018
  6. Junio C HamanoOct 14, 2018
  7. Duy NguyenOct 15, 2018
  8. Duy NguyenOct 20, 2018
  9. danaOct 20, 2018
  10. Duy NguyenOct 20, 2018
  11. danaOct 20, 2018
  12. wildmatch: change behavior of "foo**bar" in WM_PATHNAME modeNguyễn Thái Ngọc Duy, Oct 27, 2018
  13. Torsten BögershausenOct 28, 2018
  14. Duy NguyenOct 28, 2018
  15. Junio C HamanoOct 29, 2018
  16. Ævar Arnfjörð BjarmasonOct 29, 2018
  17. Duy NguyenOct 29, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.