Re: [PATCHv3 5/5] branch: allow pattern arguments

[Michael J Gruber <git@drmicha.warpmail.net>]

Michael Schubert venit, vidit, dixit 06.09.2011 15:10:
> On 08/28/2011 04:54 PM, Michael J Gruber wrote:
>> +static int match_patterns(const char **pattern, const char *refname)
>> +{
>> +	if (!*pattern)
>> +		return 1; /* no pattern always matches */
>> +	while (*pattern) {
>> +		if (!fnmatch(*pattern, refname, 0))
>> +			return 1;
>> +		pattern++;
>> +	}
>> +	return 0;
>> +}
> 
> Nitpick: maybe builtin/branch.c and builtin/tag.c could share match_pattern().?

I think all commands which list refs should share code - for-each-ref,
branch, tag, replace. I suggest to unify log and for-each-ref-formats
first, the unify ref listers.

> A second thought: the printed "remotes/" prefix could be confusing for users,
> since it seems to be part of the refname. For example:
> 
> $ git branch -a
> * master
>   maint
>   man
>   remotes/origin/master
>   remotes/origin/maint
>   remotes/origin/man
> 
> $ git branch -a --list remotes/origin*
> [no output]
> 
> but
> 
> $ git branch -a --list origin*
>   remotes/origin/master
>   remotes/origin/maint
>   remotes/origin/man
> 
> (Sorry in case I missed that) What's the reason you decided --list to show local
> branches only? Maybe --list could show all refnames without any extra prefix.?

I didn't decide anything here. "git branch" lists all local branches and
has been doing that forever, and "--list" is and should be a noop
without a pattern. That was the idea: -r/-a select refnames,
--list/-v/-vv activate the list mode(s), and the pattern is matched
against the selected refs.

(This is one reason why I didn't want it to be named --glob - the
pattern is a filter, not a selector.)

That being said, I find the fact that "-a --list remotes/origin/*" does
not match anything somewhat disconcerting, although it fits in with the
general idea of "git branch": it deals with branch names, not ref names.
Have you ever tried to delete a remote branch?

git branch -r -d origin/maint # workee
git branch -r -d remotes/origin/maint # no workee

Without -r, it doesn't work either.

So, the way it is it fits in with how "git branch" works, and is
different from for-each-ref (and rev-list --glob). I don't like it,
because you may have a local branch origin/maint (i.e.
refs/heads/origin/maint) which you can't distinguish from the remote
branch by a pattern, only by using or not using -r. But it's the same as
with "git branch -d", really.

Michael