* [PATCH] show-branch: show all local heads when only giving one rev along --topics
@ 2015-03-16 8:38 Mike Hommey
2015-03-16 23:50 ` Mike Hommey
0 siblings, 1 reply; 6+ messages in thread
From: Mike Hommey @ 2015-03-16 8:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
"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)')"
---
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 365228a..ef9e719 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.3.3.3.g6c0eb00
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] show-branch: show all local heads when only giving one rev along --topics
2015-03-16 8:38 [PATCH] show-branch: show all local heads when only giving one rev along --topics Mike Hommey
@ 2015-03-16 23:50 ` Mike Hommey
0 siblings, 0 replies; 6+ messages in thread
From: Mike Hommey @ 2015-03-16 23:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
On Mon, Mar 16, 2015 at 05:38:06PM +0900, Mike Hommey wrote:
> "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)')"
Sorry, this was missing:
Signed-off-by: Mike Hommey <mh@glandium.org>
Mike
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] show-branch: show all local heads when only giving one rev along --topics
2015-03-30 22:24 ` Junio C Hamano
2015-03-30 22:34 ` Junio C Hamano
@ 2015-03-30 22:37 ` Mike Hommey
1 sibling, 0 replies; 6+ messages in thread
From: Mike Hommey @ 2015-03-30 22:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] show-branch: show all local heads when only giving one rev along --topics
2015-03-30 22:24 ` Junio C Hamano
@ 2015-03-30 22:34 ` Junio C Hamano
2015-03-30 22:37 ` Mike Hommey
1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2015-03-30 22:34 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] show-branch: show all local heads when only giving one rev along --topics
2015-03-30 22:12 Mike Hommey
@ 2015-03-30 22:24 ` Junio C Hamano
2015-03-30 22:34 ` Junio C Hamano
2015-03-30 22:37 ` Mike Hommey
0 siblings, 2 replies; 6+ messages in thread
From: Junio C Hamano @ 2015-03-30 22:24 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] show-branch: show all local heads when only giving one rev along --topics
@ 2015-03-30 22:12 Mike Hommey
2015-03-30 22:24 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Mike Hommey @ 2015-03-30 22:12 UTC (permalink / raw)
To: gitster; +Cc: git
"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
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-30 22:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-16 8:38 [PATCH] show-branch: show all local heads when only giving one rev along --topics Mike Hommey
2015-03-16 23:50 ` Mike Hommey
2015-03-30 22:12 Mike Hommey
2015-03-30 22:24 ` Junio C Hamano
2015-03-30 22:34 ` Junio C Hamano
2015-03-30 22:37 ` Mike Hommey
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.