Re: [RFC/PATCH] tag: make list exclude !<pattern>

[Tom Grennan <tmgrennan@gmail.com>]

On Fri, Feb 10, 2012 at 11:50:06PM -0800, Junio C Hamano wrote:
>Junio C Hamano <gitster@pobox.com> writes:
>
>>    ... Same
>>    for 1c, which I highly suspect will introduce regression without
>>    looking at the code (for-each-ref is prefix-match only), ...

OK I'll study this further and run through t6300-for-each-ref.sh
I see it has a bunch of errors.
I think there are similar issues using match_pattern() with
show-branch and ls-remote.

Thanks,

>This part needs correction.  for-each-ref matches the command line
>arguments differently from branch --list and tag --list in two important
>ways.
>
> (1) It allows (not "only" which was a mistake in my earlier message)
>     prefix matching, e.g. "for-each-ref refs/heads/", in addition to
>     fnmatch(); and
>
> (2) The fnmatch() call is made with FNM_PATHMAME, which "branch --list"
>     and "tag --list" does not use.
>
>Strictly speaking, therefore, if you make all three commands to use the
>same matching logic, there is no way to avoid regression.  If you choose
>to use fnmatch() without FNM_PATHNAME, then for-each-ref suddenly starts
>matching wildcards across name hierarchy boundary '/' for a pattern that
>does not match today, e.g. "git for-each-ref 'refs/heads/*'" was a good
>way to grab only the integration branches while excluding individual topic
>branches such as refs/heads/tg/tag-points-at, but this technique can no
>longer be used for such a purpose, which is an unpleasant regression.
>
>I personally think that it was an annoying UI mistake that we let branch
>and tag call fnmatch without FNM_PATHNAME, but we cannot fix it lightly,
>either.  People who use hierchical branch names (e.g. maint-1.0/$topic,
>maint-2.0/$topic, and feature-2.0/$topic) may already be used to list all
>the topics on the maintenance tracks with "branch --list 'maint*'", and we
>need to keep "branch --list" and "tag --list" working as they expect.
>
>One possible way forward (now I am talking about a longer term solution)
>would be to introduce
>
>	refname_match_pattern(const char *refname,
>        		      const char **pattern,
>                              unsigned flags);
>
>where flags can tell the implementation if FNM_PATHNAME should be used,
>and if prefix matching should be attempted, so that the three commands
>share the single same matching function while still retaining their
>current behaviour in the initial round.  Inside the implementation, we
>would use good old fnmatch(), with or without FNM_PATHNAME, depending on
>the flags the caller passes.
>
>In a future versions, we may want to have "branch/tag --list" also ask for
>FNM_PATHNAME (this *is* a backward incompatible change, so it needs to be
>performed across major version boundary, with backward compatibility
>configurations, deprecation warnings and whole nine yards). Under the new
>match function, today's "branch --list 'maint*'" needs to be spelled as
>"branch --list 'maint*/*'" or something.  The prefix matching is probably
>safer to enable by default without causing big regression hassle if we
>limit the prefix match to only patterns that end with an explicit slash,
>as users already *know* today's "branch --list tg/" would not match
>anything (because the pattern does not even match a brahch 'tg', so it is
>unlikely they are using it and expecting only 'tg' to match), which means
>that is an unlikely input we can safely give new meaning to match anything
>under tg/ hierarchy.
>