[v2,1/1] branch: introduce --show-current display option
diff mbox series

Message ID 20181010205432.11990-2-daniels@umanovskis.se
State New
Headers show
Series
  • branch: introduce --show-current display option
Related show

Commit Message

Daniels Umanovskis Oct. 10, 2018, 8:54 p.m. UTC
When called with --show-current, git branch will print the current
branch name and terminate. Only the actual name gets printed,
without refs/heads. In detached HEAD state, nothing is output.

Intended both for scripting and interactive/informative use.
Unlike git branch --list, no filtering is needed to just get the
branch name.

Signed-off-by: Daniels Umanovskis <daniels@umanovskis.se>
---
 Documentation/git-branch.txt |  6 +++++-
 builtin/branch.c             | 21 ++++++++++++++++--
 t/t3203-branch-output.sh     | 41 ++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 3 deletions(-)

Comments

Jeff King Oct. 11, 2018, 12:34 a.m. UTC | #1
On Wed, Oct 10, 2018 at 10:54:32PM +0200, Daniels Umanovskis wrote:

> When called with --show-current, git branch will print the current
> branch name and terminate. Only the actual name gets printed,
> without refs/heads. In detached HEAD state, nothing is output.

I also wondered what happens in an unborn-branch state (i.e., we are on
refs/heads/master, but have not yet made any commits).

In that case, resolve_ref_unsafe() will return the branch name, but a
null oid. And you'll print that. Which seems sensible.

> Intended both for scripting and interactive/informative use.
> Unlike git branch --list, no filtering is needed to just get the
> branch name.

We should not advertise this to be used for scripting. The git-branch
command is porcelain, and we reserve the right to change its output
between versions, or based on user config. Fortunately, there is already
a blessed way to get this in a script, which is:

  git symbolic-ref [--short] HEAD

I'm not opposed to having "branch --show-current" as a more user-facing
alternative for people who want to query the state. I do wonder if
people would want it to show extra information, like:

  - if we're detached, from where (like the normal "branch --list"
    shows)

  - are we on an unborn branch

  - are we ahead/behind an upstream (like "branch -v")

I guess that's slowly reinventing the first line of "git status -b".
Maybe that's a good thing; possibly this should just be a way to get
that data without doing the rest of git-status, and it's perhaps more
discoverable since it's part of git-branch. I dunno.

It just seems like in its current form it might be in an uncanny valley
where it is not quite scriptable plumbing, but not as informative as
other porcelain.

-Peff
Junio C Hamano Oct. 11, 2018, 6:54 a.m. UTC | #2
Daniels Umanovskis <daniels@umanovskis.se> writes:

> +static void print_current_branch_name()
> +{
> +	const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
> +	const char *shortname;
> +	if (refname == NULL || !strcmp(refname, "HEAD"))
> +		return;

Is it a normal situation to have refname==NULL, or is it something
worth reporting as an error?

Without passing the &flag argument, I do not think there is a
reliable way to ask resolve_ref_unsafe() if "HEAD" is a symbolic
ref.

	int flag;
	const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, &flag);
	const char *branchname;

	if (!refname)
		die(...);
	else if (!(flag & REF_ISSYMREF))
		return; /* detached HEAD */
	else if (skip_prefix(refname, "refs/heads/", &branchname))
		puts(branchname);
	else
		die("HEAD (%s) points outside refs/heads/?", refname);

or something like that?
Rafael Ascensão Oct. 11, 2018, 3:43 p.m. UTC | #3
On Wed, Oct 10, 2018 at 08:34:40PM -0400, Jeff King wrote:
> It just seems like in its current form it might be in an uncanny valley
> where it is not quite scriptable plumbing, but not as informative as
> other porcelain.

I agree it feels a bit out of place, and still think that

    $ git branch --list HEAD

would be a good candidate to be taught how to print the current branch.

I suggested this in the previous iteration but either got lost in the
noise or was uninteresting. If the latter, I would love to receive
feedback on it.
https://public-inbox.org/git/20181010142423.GA3390@rigel/

Something like the following (not meant as a real patch), would show the
current branch when attached, (HEAD detached at hash) when detached, and
nothing if unborn branch.

This will also keep the current formatting git branch uses (which is
sliglty harder to parse). I view it as a plus. Otherwise people will
eventually start parsing it instead of using the recommended plumbing.

--
Cheers
Rafael Ascensão

-- >8 --
diff --git a/builtin/branch.c b/builtin/branch.c
index b67593288c..78a3de526c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -684,6 +684,17 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
 			filter.kind |= FILTER_REFS_DETACHED_HEAD;
 		filter.name_patterns = argv;
+
+		while (*argv) {
+			if (!strcmp(*argv, "HEAD")) {
+				const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
+				skip_prefix(refname, "refs/heads/", &refname);
+				filter.name_patterns[argv - filter.name_patterns] = refname;
+				break;
+			}
+			argv++;
+		}
+
 		/*
 		 * If no sorting parameter is given then we default to sorting
 		 * by 'refname'. This would give us an alphabetically sorted
Daniels Umanovskis Oct. 11, 2018, 4:36 p.m. UTC | #4
On 10/11/18 5:43 PM, Rafael Ascensão wrote:
> I agree it feels a bit out of place, and still think that
> 
>     $ git branch --list HEAD
> 
> would be a good candidate to be taught how to print the current branch.

I am not a fan because it would be yet another inconsistency in the Git
command interface. An argument given after git branch --list means a
pattern for the branches to list. Making HEAD print the current branch
would be an exception to what an argument in that place means. Yes, HEAD
itself is a very special string in git, but I'm not a fan of syntax
where a specific argument value does something very different from any
other value in that place.
Daniels Umanovskis Oct. 11, 2018, 5:29 p.m. UTC | #5
On 10/11/18 8:54 AM, Junio C Hamano wrote:
> Is it a normal situation to have refname==NULL, or is it something
> worth reporting as an error?

Looks like that would be in the case of looping symrefs or file backend
failure, so seems a good idea to die() in that case.

> Without passing the &flag argument, I do not think there is a
> reliable way to ask resolve_ref_unsafe() if "HEAD" is a symbolic
> ref.

If I'm reading the code correctly, resolve_ref_unsafe() will return
"HEAD" or NULL if there's no symbolic reference, so anything else would
indicate a symref, but even in that case checking the flag explicitly is
definitely better to clearly show intent.

Will soon reply with v3 cleaning up the suggested patch accordingly.
Jeff King Oct. 11, 2018, 5:51 p.m. UTC | #6
On Thu, Oct 11, 2018 at 06:36:02PM +0200, Daniels Umanovskis wrote:

> On 10/11/18 5:43 PM, Rafael Ascensão wrote:
> > I agree it feels a bit out of place, and still think that
> > 
> >     $ git branch --list HEAD
> > 
> > would be a good candidate to be taught how to print the current branch.
> 
> I am not a fan because it would be yet another inconsistency in the Git
> command interface. An argument given after git branch --list means a
> pattern for the branches to list. Making HEAD print the current branch
> would be an exception to what an argument in that place means. Yes, HEAD
> itself is a very special string in git, but I'm not a fan of syntax
> where a specific argument value does something very different from any
> other value in that place.

Yeah, I agree. If we were to go this route, it should probably be:

  git branch --list-head

Which sounds a lot like what you are proposing, but I think the name
implies more strongly "show --list, but only for the HEAD". I.e., for
the detached case, show the "HEAD detached at..." text.

-Peff
Jeff King Oct. 11, 2018, 5:52 p.m. UTC | #7
On Thu, Oct 11, 2018 at 07:29:58PM +0200, Daniels Umanovskis wrote:

> > Without passing the &flag argument, I do not think there is a
> > reliable way to ask resolve_ref_unsafe() if "HEAD" is a symbolic
> > ref.
> 
> If I'm reading the code correctly, resolve_ref_unsafe() will return
> "HEAD" or NULL if there's no symbolic reference, so anything else would
> indicate a symref, but even in that case checking the flag explicitly is
> definitely better to clearly show intent.

Yes, that matches my understanding, too.

-Peff
Rafael Ascensão Oct. 11, 2018, 8:35 p.m. UTC | #8
On Thu, Oct 11, 2018 at 06:36:02PM +0200, Daniels Umanovskis wrote:
> I am not a fan because it would be yet another inconsistency in the Git
> command interface.

The output of the proposed command is also a bit inconsistent with the
usual output given by git branch, specifically the space alignment on
the left, color and * marker.

In addition to not respecting --color, it also ignores --verbose and
--format. At this stage it's closer to what I would expect from
$git rev-parse --abbrev-ref HEAD; than something coming out of
$git branch; Resolving HEAD makes it consistent with rest.

On Thu, Oct 11, 2018 at 01:51:36PM -0400, Jeff King wrote:
> Yeah, I agree.

Not sure which parts you meant, so I'll assume you didn't agree
with me.

I doesn't seem far fetched to ask for an overview of my current branch,
feature1, feature2 and all hotfixes with something like:

  $ git branch --verbose --list HEAD feature1 feature2 hotfix-*;

The 'what's my current branch' could be just a particular case of this
form.

My defense to treat HEAD specially comes in the form that from the user
perspective, HEAD is already being resolved to a commit when HEAD is
detached (Showing the detached at <hash> message.)

Is there a strong reason to *not* "resolve" HEAD when it is attached?
Would it be that bad to have some DWIM behaviour here? After all, as
HEAD is an invalid name for a branch, nothing would ever match it
anyways.


Thanks for the input. :)
--
Cheers
Rafael Ascensão
Daniels Umanovskis Oct. 11, 2018, 8:46 p.m. UTC | #9
On 10/11/18 10:35 PM, Rafael Ascensão wrote:
> The output of the proposed command is also a bit inconsistent with the
> usual output given by git branch, specifically the space alignment on
> the left, color and * marker.

The proposed command therefore takes a new switch. It's definitely not
perfect, but doesn't give the existing --list new and different behavior.

> At this stage it's closer to what I would expect from
> $git rev-parse --abbrev-ref HEAD;

The proposal is largely to have similar output to that command, yes. I
expect that "show current branch" is something that's available in the
branch command, even completely disregarding questions of whether it's
API stable, etc.
Jeff King Oct. 11, 2018, 8:53 p.m. UTC | #10
On Thu, Oct 11, 2018 at 09:35:28PM +0100, Rafael Ascensão wrote:

> On Thu, Oct 11, 2018 at 01:51:36PM -0400, Jeff King wrote:
> > Yeah, I agree.
> 
> Not sure which parts you meant, so I'll assume you didn't agree
> with me.

Correct. ;)

I like your general idea, but I agree with Daniel that it introduces an
inconsistency in the interface.

> I doesn't seem far fetched to ask for an overview of my current branch,
> feature1, feature2 and all hotfixes with something like:
> 
>   $ git branch --verbose --list HEAD feature1 feature2 hotfix-*;
> 
> The 'what's my current branch' could be just a particular case of this
> form.

Right, I like that part. It's just that putting "HEAD" there already has
a meaning: it would find refs/heads/HEAD.

Now I'll grant that's a bad name for a branch (and the source of other
confusions, and I think perhaps even something a few commands actively
discourage these days).

> My defense to treat HEAD specially comes in the form that from the user
> perspective, HEAD is already being resolved to a commit when HEAD is
> detached (Showing the detached at <hash> message.)
> 
> Is there a strong reason to *not* "resolve" HEAD when it is attached?
> Would it be that bad to have some DWIM behaviour here? After all, as
> HEAD is an invalid name for a branch, nothing would ever match it
> anyways.

I don't think this is about resolving HEAD, or showing it. It's about
the fact that arguments to "branch" are currently always branch-names,
not full refs.

-Peff
Rafael Ascensão Oct. 11, 2018, 10:34 p.m. UTC | #11
On Thu, Oct 11, 2018 at 04:53:23PM -0400, Jeff King wrote:
> Right, I like that part. It's just that putting "HEAD" there already has
> a meaning: it would find refs/heads/HEAD.
> 
> Now I'll grant that's a bad name for a branch (and the source of other
> confusions, and I think perhaps even something a few commands actively
> discourage these days).
>

Makes sense. My whole premise was based on the fact that refs/heads/HEAD
wouldn't be supported. Now it's obvious to me that isn't necessarily
true. And now I understand the real issue. Thanks for bearing with me.

I also agree with your proposed '--list-head' suggestion.

So, ideally, instead of my broken suggestion of:
    $ git branch --verbose --list HEAD feature1 hotfix-*;

The equivalent would be:
    $ git branch --verbose --list-head --list feature1 hotfix-*;

and it would coalesce nicely as long as --list-head conforms with the
default formatting for --list.

--
Cheers
Rafael Ascensão
SZEDER Gábor Oct. 11, 2018, 10:53 p.m. UTC | #12
On Wed, Oct 10, 2018 at 10:54:32PM +0200, Daniels Umanovskis wrote:

[...]

> diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
> index ee6787614..e9bc3b05f 100755
> --- a/t/t3203-branch-output.sh
> +++ b/t/t3203-branch-output.sh
> @@ -100,6 +100,47 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '
>  	test_must_fail git branch -v branch*
>  '
>  
> +test_expect_success 'git branch `--show-current` shows current branch' '
> +	cat >expect <<-\EOF &&
> +	branch-two
> +	EOF
> +	git checkout branch-two &&

Up to this point everything talked about '--show-current' ...

> +	git branch --current >actual &&

... but here and in all the following tests you run

  git branch --current

which then fails with "error: unknown option `current'"

> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git branch `--show-current` is silent when detached HEAD' '
> +	git checkout HEAD^0 &&
> +	git branch --current >actual &&
> +	test_must_be_empty actual
> +'
> +
> +test_expect_success 'git branch `--show-current` works properly when tag exists' '
> +	cat >expect <<-\EOF &&
> +	branch-and-tag-name
> +	EOF
> +	git checkout -b branch-and-tag-name &&
> +	git tag branch-and-tag-name &&
> +	git branch --current >actual &&
> +	git checkout branch-one &&
> +	git branch -d branch-and-tag-name &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git branch `--show-current` works properly with worktrees' '
> +	cat >expect <<-\EOF &&
> +	branch-one
> +	branch-two
> +	EOF
> +	git checkout branch-one &&
> +	git branch --current >actual &&
> +	git worktree add worktree branch-two &&
> +	cd worktree &&
> +	git branch --current >>../actual &&
> +	cd .. &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'git branch shows detached HEAD properly' '
>  	cat >expect <<EOF &&
>  * (HEAD detached at $(git rev-parse --short HEAD^0))
> -- 
> 2.19.1.330.g93276587c.dirty
>
SZEDER Gábor Oct. 11, 2018, 10:56 p.m. UTC | #13
On Fri, Oct 12, 2018 at 12:53:26AM +0200, SZEDER Gábor wrote:
> On Wed, Oct 10, 2018 at 10:54:32PM +0200, Daniels Umanovskis wrote:
> 
> [...]
> 
> > diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
> > index ee6787614..e9bc3b05f 100755
> > --- a/t/t3203-branch-output.sh
> > +++ b/t/t3203-branch-output.sh
> > @@ -100,6 +100,47 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '
> >  	test_must_fail git branch -v branch*
> >  '
> >  
> > +test_expect_success 'git branch `--show-current` shows current branch' '
> > +	cat >expect <<-\EOF &&
> > +	branch-two
> > +	EOF
> > +	git checkout branch-two &&
> 
> Up to this point everything talked about '--show-current' ...
> 
> > +	git branch --current >actual &&
> 
> ... but here and in all the following tests you run
> 
>   git branch --current
> 
> which then fails with "error: unknown option `current'"

Ah, OK, just noticed v3 which has already fixed this.
Daniels Umanovskis Oct. 11, 2018, 10:58 p.m. UTC | #14
On 10/12/18 12:56 AM, SZEDER Gábor wrote:
> Ah, OK, just noticed v3 which has already fixed this.
> 
Yeah - squashed the wrong commits locally for v2. Thanks for pointing
this out anyway!

Patch
diff mbox series

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index bf5316ffa..0babb9b1b 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -9,7 +9,7 @@  SYNOPSIS
 --------
 [verse]
 'git branch' [--color[=<when>] | --no-color] [-r | -a]
-	[--list] [-v [--abbrev=<length> | --no-abbrev]]
+	[--list] [--show-current] [-v [--abbrev=<length> | --no-abbrev]]
 	[--column[=<options>] | --no-column] [--sort=<key>]
 	[(--merged | --no-merged) [<commit>]]
 	[--contains [<commit]] [--no-contains [<commit>]]
@@ -160,6 +160,10 @@  This option is only applicable in non-verbose mode.
 	branch --list 'maint-*'`, list only the branches that match
 	the pattern(s).
 
+--show-current::
+	Print the name of the current branch. In detached HEAD state,
+	nothing is printed.
+
 -v::
 -vv::
 --verbose::
diff --git a/builtin/branch.c b/builtin/branch.c
index c396c4153..ab03073b2 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -443,6 +443,17 @@  static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
 	free(to_free);
 }
 
+static void print_current_branch_name()
+{
+	const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
+	const char *shortname;
+	if (refname == NULL || !strcmp(refname, "HEAD"))
+		return;
+	if (!skip_prefix(refname, "refs/heads/", &shortname))
+		die(_("unexpected symbolic ref for HEAD: %s"), refname);
+	puts(shortname);
+}
+
 static void reject_rebase_or_bisect_branch(const char *target)
 {
 	struct worktree **worktrees = get_worktrees(0);
@@ -581,6 +592,7 @@  static int edit_branch_description(const char *branch_name)
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
 	int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
+	int show_current = 0;
 	int reflog = 0, edit_description = 0;
 	int quiet = 0, unset_upstream = 0;
 	const char *new_upstream = NULL;
@@ -620,6 +632,7 @@  int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('c', "copy", &copy, N_("copy a branch and its reflog"), 1),
 		OPT_BIT('C', NULL, &copy, N_("copy a branch, even if target exists"), 2),
 		OPT_BOOL('l', "list", &list, N_("list branch names")),
+		OPT_BOOL(0, "show-current", &show_current, N_("show current branch name")),
 		OPT_BOOL(0, "create-reflog", &reflog, N_("create the branch's reflog")),
 		OPT_BOOL(0, "edit-description", &edit_description,
 			 N_("edit the description for the branch")),
@@ -662,14 +675,15 @@  int cmd_branch(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 			     0);
 
-	if (!delete && !rename && !copy && !edit_description && !new_upstream && !unset_upstream && argc == 0)
+	if (!delete && !rename && !copy && !edit_description && !new_upstream &&
+	    !show_current && !unset_upstream && argc == 0)
 		list = 1;
 
 	if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr ||
 	    filter.no_commit)
 		list = 1;
 
-	if (!!delete + !!rename + !!copy + !!new_upstream +
+	if (!!delete + !!rename + !!copy + !!new_upstream + !!show_current +
 	    list + unset_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
@@ -697,6 +711,9 @@  int cmd_branch(int argc, const char **argv, const char *prefix)
 		if (!argc)
 			die(_("branch name required"));
 		return delete_branches(argc, argv, delete > 1, filter.kind, quiet);
+	} else if (show_current) {
+		print_current_branch_name();
+		return 0;
 	} else if (list) {
 		/*  git branch --local also shows HEAD when it is detached */
 		if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index ee6787614..e9bc3b05f 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -100,6 +100,47 @@  test_expect_success 'git branch -v pattern does not show branch summaries' '
 	test_must_fail git branch -v branch*
 '
 
+test_expect_success 'git branch `--show-current` shows current branch' '
+	cat >expect <<-\EOF &&
+	branch-two
+	EOF
+	git checkout branch-two &&
+	git branch --current >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git branch `--show-current` is silent when detached HEAD' '
+	git checkout HEAD^0 &&
+	git branch --current >actual &&
+	test_must_be_empty actual
+'
+
+test_expect_success 'git branch `--show-current` works properly when tag exists' '
+	cat >expect <<-\EOF &&
+	branch-and-tag-name
+	EOF
+	git checkout -b branch-and-tag-name &&
+	git tag branch-and-tag-name &&
+	git branch --current >actual &&
+	git checkout branch-one &&
+	git branch -d branch-and-tag-name &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git branch `--show-current` works properly with worktrees' '
+	cat >expect <<-\EOF &&
+	branch-one
+	branch-two
+	EOF
+	git checkout branch-one &&
+	git branch --current >actual &&
+	git worktree add worktree branch-two &&
+	cd worktree &&
+	git branch --current >>../actual &&
+	cd .. &&
+	test_cmp expect actual
+'
+
 test_expect_success 'git branch shows detached HEAD properly' '
 	cat >expect <<EOF &&
 * (HEAD detached at $(git rev-parse --short HEAD^0))