Re: [PATCH] show-branch: show all local heads when only giving one rev along --topics

[Mike Hommey <mh@glandium.org>]

On Mon, Mar 30, 2015 at 03:24:26PM -0700, Junio C Hamano wrote:
> Mike Hommey <mh@glandium.org> writes:
> 
> > (Maybe --topics should always require one rev on the command
> > line?)
> 
> That sounds line a good thing to do.
> 
> > -	else if (all_heads + all_remotes)
> > -		snarf_refs(all_heads, all_remotes);
> >  	else {
> >  		while (0 < ac) {
> >  			append_one_rev(*av);
> >  			ac--; av++;
> >  		}
> > +		if (all_heads + all_remotes)
> > +			snarf_refs(all_heads, all_remotes);
> 
> Hmmmmmm.  Is this safe and will not cause problems by possibly
> duplicated refnames that came from the command line and the ones
> that came from for-each-ref iteration?  I am not saying the change
> is problematic; it is just I haven't looked at this code for a long
> time that the existing machinery is already designed to tolerate
> duplicated input.

It is:
https://github.com/git/git/blob/master/builtin/show-branch.c#L382

In case you wonder about allow_dups, the only case in which it's 1 is:
https://github.com/git/git/blob/master/builtin/show-branch.c#L784
which is the reflog case, which is the case before that `else` in the
patch.

That is, both append_one_rev and snarf_refs end up calling append_ref
with allow_dups=0.

Cheers,

Mike