Re: [PATCH 1/2] rev-parse --glob

[Ilari Liusvaara <ilari.liusvaara@elisanet.fi>]

On Wed, Jan 20, 2010 at 02:17:15AM -0800, Junio C Hamano wrote:
> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
> 
> Looks fairly competently done.  Does this mean you've abandoned the --namespace
> series?  I don't mind reverting them from 'next'.

Pretty much.
 
> > +static int filter_refs(const char* ref, const unsigned char *sha, int flags,
> > +	void* data)
> > +{
> > +	struct ref_filter *filter = (struct ref_filter*)data;
> > +	if (fnmatch(filter->pattern, ref, 0))
> > +		return 0;
> 
> In many places (not just refs) we use "If leading path component matches,
> or fnmatch() says Ok" (e.g. builtin-for-each-ref.c, builtin_grep.c), and
> in some other places we only use fnmatch().  I think it is better to pick
> one convention, and adjust the other one to match, and my gut feeling is
> that "leading path or fnmatch()", while theologically might be messier,
> would be more useful in practice.  And that was why I did my "like this"
> patch http://article.gmane.org/gmane.comp.version-control.git/137492 that way.

That pattern of course has been edited before (and the namespace behavoiour
is implemented that way).

> My longer term desire is to consolidate these fnmatch() users to call a
> single helper function, and then perhaps replace fnmatch() in that helper
> function with something more powerful.  Although I do not personally care
> too much, people often wanted to have "path/**/leaf-that-is-deep" matcher.

Extended globs? Except that:

'git check-ref-format refs/heads/X+\(a\|b\) ; echo $?' => 0

And '+(a|b)' is wildcard in extended glob (not expressible with standard
glob operators). 

Extended globbing in gitignore would be useful as some have asked if there
is way to use regular expressions there (nope). At least extended globs
are equivalent to regular expressions in power.

 
> > @@ -674,6 +691,35 @@ int for_each_replace_ref(each_ref_fn fn, void *cb_data)
> >  	return do_for_each_ref("refs/replace/", fn, 13, 0, cb_data);
> >  }
> >  
> > +int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data)
> > +{
> > +	struct strbuf real_pattern = STRBUF_INIT;
> > +	struct ref_filter filter;
> > +	const char *has_glob_specials;
> > +	int ret;
> > +
> > +	if (prefixcmp(pattern, "refs/"))
> > +		strbuf_addstr(&real_pattern, "refs/");
> > +	strbuf_addstr(&real_pattern, pattern);
> > +
> > +	has_glob_specials = strpbrk(pattern, "?*[");
> 
> Would the need for this code hopefully disappear if we take the "leading
> path or fnmatch()" check?  If we still need this, we probably would want
> to add a has_glob_special() helper function not as a hardcoded srpbrk(),
> but in terms of is_glob_special() in git-compat-util.h.

'--glob=remotes/origin' and '--glob=remotes/origin*' are not the same. The
first gets DWIMed to '--glob=refs/remotes/origin/*'. The usual case for
globbing is globbing entiere namespace (name of one remote can be prefix
of another).

-Ilari