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

4 messages from 2015-03-30 to 2015-03-30. Participants: Mike Hommey, Junio C Hamano.
Thread: https://gitlist.dev/t/38949

## Mike Hommey, 2015-03-30 22:12

Subject: [PATCH] show-branch: show all local heads when only giving one rev along --topics
Message-ID: <1427753543-19680-1-git-send-email-mh@glandium.org>
URL: https://gitlist.dev/e/1427753543-19680-1-git-send-email-mh%40glandium.org

```
"git show-branch --topics <rev> <revs>..." displays ancestry graph, only
considering commits that are in all given revs, except the first one.

"git show-branch" displays ancestry graph for all local branches.

Unfortunately, "git show-branch --topics <rev>" only prints out the rev
info for the given rev, and nothing else, e.g.:

  $ git show-branch --topics origin/master
  [origin/master] Sync with 2.3.3

While there is an option to add all remote-tracking branches (-r), and
another to add all local+remote branches (-a), there is no option to add
only local branches. Adding such an option could be considered, but a
user would likely already expect that the above command line considers
the lack of rev other than for --topics as meaning all local branches,
like when there is no argument at all.

Moreover, when using -r and -a along with --topics, the first local or
remote-tracking branch, depending on alphabetic order is used instead of
the one given after --topics (any rev given on the command line is
actually simply ignored when either -r or -a is given). And if no rev is
given at all, the fact that the first alphetical branch is the base of
topics is probably not expected by users (Maybe --topics should always
require one rev on the command line?)

This change makes
  "show-branch --topics $rev"
act as
  "show-branch --topics $rev $(git for-each-ref refs/heads
                               --format='%(refname:short)')"

  "show-branch -r --topics $rev ..."
act as
  "show-branch --topics $rev ... $(git for-each-ref refs/remotes
                                   --format='%(refname:short)')"
instead of
  "show-branch --topics $(git for-each-ref refs/remotes
                          --format='%(refname:short)')"

and
  "show-branch -a --topics $rev ..."
act as
  "show-branch --topics $rev ... $(git for-each-ref refs/heads refs/remotes
                                   --format='%(refname:short)')"
instead of
  "show-branch --topics $(git for-each-ref refs/heads refs/remotes
                          --format='%(refname:short)')"

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 builtin/show-branch.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index f3fb5fb..e69fb7c 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -718,7 +718,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 	}
 
 	/* If nothing is specified, show all branches by default */
-	if (ac + all_heads + all_remotes == 0)
+	if (ac <= topics && all_heads + all_remotes == 0)
 		all_heads = 1;
 
 	if (reflog) {
@@ -785,13 +785,13 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 		}
 		free(ref);
 	}
-	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);
 	}
 
 	head_p = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
-- 
2.4.0.rc0.1.g7f94b17

```

## Junio C Hamano, 2015-03-30 22:24

Subject: Re: [PATCH] show-branch: show all local heads when only giving one rev along --topics
Message-ID: <xmqqd23qw2cl.fsf@gitster.dls.corp.google.com>
URL: https://gitlist.dev/e/xmqqd23qw2cl.fsf%40gitster.dls.corp.google.com
In-Reply-To: <1427753543-19680-1-git-send-email-mh@glandium.org>

```
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.

Thanks.

```

## Junio C Hamano, 2015-03-30 22:34

Subject: Re: [PATCH] show-branch: show all local heads when only giving one rev along --topics
Message-ID: <xmqq8ueew1wc.fsf@gitster.dls.corp.google.com>
URL: https://gitlist.dev/e/xmqq8ueew1wc.fsf%40gitster.dls.corp.google.com
In-Reply-To: <xmqqd23qw2cl.fsf@gitster.dls.corp.google.com>

```
Junio C Hamano <gitster@pobox.com> writes:

> ...?  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.

"for a long time to say that the existing code is OK or not" is what
I meant to say.  Sorry for the noise.

> Thanks.

```

## Mike Hommey, 2015-03-30 22:37

Subject: Re: [PATCH] show-branch: show all local heads when only giving one rev along --topics
Message-ID: <20150330223752.GA19987@glandium.org>
URL: https://gitlist.dev/e/20150330223752.GA19987%40glandium.org
In-Reply-To: <xmqqd23qw2cl.fsf@gitster.dls.corp.google.com>

```
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

```