Junio C Hamano <gitster@xxxxxxxxx> writes:
> ... Same
> for 1c, which I highly suspect will introduce regression without
> looking at the code (for-each-ref is prefix-match only), ...
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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html