git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 1/2] branch: introduce --current display option
@ 2018-10-09 18:31 Daniels Umanovskis
  2018-10-09 18:31 ` [PATCH 2/2] doc/git-branch: Document the --current option Daniels Umanovskis
  2018-10-09 19:54 ` [PATCH 1/2] branch: introduce --current display option Stefan Beller
  0 siblings, 2 replies; 5+ messages in thread
From: Daniels Umanovskis @ 2018-10-09 18:31 UTC (permalink / raw)
  To: git; +Cc: Daniels Umanovskis

When called with --current, git branch will print the current
branch name and terminate. It will print HEAD in detached-head state.

Rationale: finding out the current branch is useful interactively,
but especially in scripting. git branch --list prints many branches,
and prepends the current one with an asterisk, meaning sed or other
filtering is necessary to just get the current branch.
git rev-parse --abbrev-ref HEAD is the current way to achieve this
output, but that is not intuitive or easy to understand.

Signed-off-by: Daniels Umanovskis <daniels@umanovskis.se>
---
 builtin/branch.c         | 17 +++++++++++++++++
 t/t3203-branch-output.sh | 18 ++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/builtin/branch.c b/builtin/branch.c
index c396c4153..e4c6b0490 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -443,6 +443,18 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
 	free(to_free);
 }
 
+static void print_current_branch_name()
+{
+	struct strbuf out = STRBUF_INIT;
+	const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
+	char *shortname = shorten_unambiguous_ref(refname, 0);
+	strbuf_addf(&out, _("%s"), shortname);
+	fwrite(out.buf, 1, out.len, stdout);
+	putchar('\n');
+	free(shortname);
+	strbuf_release(&out);
+}
+
 static void reject_rebase_or_bisect_branch(const char *target)
 {
 	struct worktree **worktrees = get_worktrees(0);
@@ -581,6 +593,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 current = 0;
 	int reflog = 0, edit_description = 0;
 	int quiet = 0, unset_upstream = 0;
 	const char *new_upstream = NULL;
@@ -620,6 +633,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, "current", &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")),
@@ -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 (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..396d81568 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -100,6 +100,24 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '
 	test_must_fail git branch -v branch*
 '
 
+test_expect_success 'git branch `--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 `--current` shows detached HEAD properly' '
+	cat >expect <<-\EOF &&
+	HEAD
+	EOF
+    git checkout HEAD^0 &&
+	git branch --current >actual &&
+	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.274.g059d67db4.dirty


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] doc/git-branch: Document the --current option
  2018-10-09 18:31 [PATCH 1/2] branch: introduce --current display option Daniels Umanovskis
@ 2018-10-09 18:31 ` Daniels Umanovskis
  2018-10-10  2:48   ` Junio C Hamano
  2018-10-09 19:54 ` [PATCH 1/2] branch: introduce --current display option Stefan Beller
  1 sibling, 1 reply; 5+ messages in thread
From: Daniels Umanovskis @ 2018-10-09 18:31 UTC (permalink / raw)
  To: git; +Cc: Daniels Umanovskis

Signed-off-by: Daniels Umanovskis <daniels@umanovskis.se>
---
 Documentation/git-branch.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index bf5316ffa..a7167df74 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] [--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).
 
+--current::
+	Print the name of the current branch. In detached HEAD state,
+	or if otherwise impossible to resolve the branch name, print
+	"HEAD".
 -v::
 -vv::
 --verbose::
-- 
2.19.1.274.g059d67db4.dirty


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] branch: introduce --current display option
  2018-10-09 18:31 [PATCH 1/2] branch: introduce --current display option Daniels Umanovskis
  2018-10-09 18:31 ` [PATCH 2/2] doc/git-branch: Document the --current option Daniels Umanovskis
@ 2018-10-09 19:54 ` Stefan Beller
  2018-10-09 20:21   ` Daniels Umanovskis
  1 sibling, 1 reply; 5+ messages in thread
From: Stefan Beller @ 2018-10-09 19:54 UTC (permalink / raw)
  To: daniels; +Cc: git

Welcome to the git mailing list!

On Tue, Oct 9, 2018 at 11:31 AM Daniels Umanovskis
<daniels@umanovskis.se> wrote:
>
> When called with --current, git branch will print the current
> branch name and terminate. It will print HEAD in detached-head state.

How does it play with worktrees? (I would expect it to just work as expected:
each worktree would print its current branch, but a test might help?)

> Rationale: finding out the current branch is useful interactively,
> but especially in scripting. git branch --list prints many branches,
> and prepends the current one with an asterisk, meaning sed or other
> filtering is necessary to just get the current branch.
> git rev-parse --abbrev-ref HEAD is the current way to achieve this
> output, but that is not intuitive or easy to understand.

Git used to have (and still has) the approach of dividing its commands
into high level ("porcelain") commands and low level ("plumbing") commands,
with the porcelain facing the user and plumbing being good for scripting.

This patch proposes a radically different approach, which is convenience
of use.

As a scripter you'd need to find out if "branch --current" is stable in its API
or if you'd rather parse it out of the branch list, which adds a subtle burden
to the scripter, as it is convenient to leave out that part and just use what is
there. :-)

> +static void print_current_branch_name()
> +{
> +       struct strbuf out = STRBUF_INIT;
> +       const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
> +       char *shortname = shorten_unambiguous_ref(refname, 0);


> +       strbuf_addf(&out, _("%s"), shortname);
> +       fwrite(out.buf, 1, out.len, stdout);

Why do we need to add the shortname to a strbuf?
(and using _( ) that denotes the string should be translated?)
I would think we could just

    puts(shortname)

here and leave out all the strbuf out ?


> @@ -620,6 +633,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, "current", &current, N_("show current branch name")),

(Gah, we're not using OPT_MODE here to select for one out of many.)

Later we have it in code via

    if (!!delete + !!rename + !!new_upstream +
        list + unset_upstream > 1)
            usage_with_options(builtin_branch_usage, options);

and I would think we'd want to add in a "+ !!current" there, too.
Then we'd get the usage options when giving --current in combination
with say --move

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] branch: introduce --current display option
  2018-10-09 19:54 ` [PATCH 1/2] branch: introduce --current display option Stefan Beller
@ 2018-10-09 20:21   ` Daniels Umanovskis
  0 siblings, 0 replies; 5+ messages in thread
From: Daniels Umanovskis @ 2018-10-09 20:21 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git

Thanks for the feedback!

On 10/9/18 9:54 PM, Stefan Beller wrote:
> How does it play with worktrees? (I would expect it to just work as expected:
> each worktree would print its current branch, but a test might help?)

I'll see about writing a test for that. I've never used git-worktree so
this is a good chance to learn.

> Git used to have (and still has) the approach of dividing its commands
> into high level ("porcelain") commands and low level ("plumbing") commands,
> with the porcelain facing the user and plumbing being good for scripting.
> 
> This patch proposes a radically different approach, which is convenience
> of use.

Right - a couple of points in response. First, it strikes me that "print
current branch" is one of those things that are both for scripting and
for normal/interactive use. Much like the entirety of git-status, which
is very used by itself, and also in scripts with --porcelain. Current
branch is often used in things like your shell prompt, vim statusline,
etc, as well as scripts.

The amount of upvotes on the stackoverflow question "how to get the
current branch name in git?" illustrates my point :)

Second, it seems arguable whether `git ref-parse` is guaranteed to be
scripting-safe, it's not actually a plumbing command, at least if we
trust the main git manpage, which lists git-rev-parse as porcelain anyway.

> Why do we need to add the shortname to a strbuf?
> (and using _( ) that denotes the string should be translated?)
> I would think we could just
> 
>     puts(shortname)
> 
> here and leave out all the strbuf out ?
> 
Of course, has to be changed for v2. Pitfalls of learning the code from
just its surroundings, didn't realize that the underscores are for i18n.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] doc/git-branch: Document the --current option
  2018-10-09 18:31 ` [PATCH 2/2] doc/git-branch: Document the --current option Daniels Umanovskis
@ 2018-10-10  2:48   ` Junio C Hamano
  0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2018-10-10  2:48 UTC (permalink / raw)
  To: Daniels Umanovskis; +Cc: git

Daniels Umanovskis <daniels@umanovskis.se> writes:

> +--current::
> +	Print the name of the current branch. In detached HEAD state,
> +	or if otherwise impossible to resolve the branch name, print
> +	"HEAD".

Where does "if otherwise impossible to resolve" come from?  In the
code in [PATCH 1/2], we see this bit

+	const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
+	char *shortname = shorten_unambiguous_ref(refname, 0);

and the output phase would become puts(shortname).

 * Under what condition resolve_ref_unsafe(HEAD) fail to resolve,
   and when that happens what does it return?  "HEAD"?  Can the
   caller tell the case in which .git/HEAD is a symref that points
   at refs/heads/HEAD (i.e. we are on a branch whose name is "HEAD")
   and the case in which .git/HEAD fails to resolve and you get
   "HEAD" back?

 * Or does the function return NULL in "otherwise impossible" case?
   Does shorten_unambiguous_ref() deal with refname==NULL
   gracefully?

 * Under what condition shorten_unambiguous_ref() fail to compute
   the branch name discovered by resolve_ref_unsafe()?

Also, I do not think the implementation is correct.  When you are on
the 'frotz' branch, and if you happen to have a tag whose name also
is 'frotz', then

 - Your .git/HEAD points at refs/heads/frotz, and refs/heads/frotz
   is what resolve_ref_unsafe() gives you.

 - You have refs/heads/frotz and refs/tags/frotz in the repository.
   Asking to shorten the ref refs/heads/frotz unambiguously will
   *not* yield 'frotz'.  It will give you something like 'heads/frotz'
   to avoid getting it confused with tags/frotz

 - Still "git branch --list" would show 'frotz' in such a case, and
   your "--current" would definitely want to match the behaviour.

I think the correct implementation should be more like:

 - Ask resolve-ref-unsafe about HEAD; if it is not a symbolic ref,
   then we are on a detached HEAD.  Silently exit with status 0.

 - If it is a symbolic ref, see if the target of the symblic ref
   (i.e. returned refname) begins with "refs/heads/".  Otherwise, we
   have a repository corruption.  Diagnose it as an error and die().

 - Otherwise, strip that leading "refs/heads/"; the remainder is the
   name of the "current branch".

I already said "current" by itself is an unacceptable name for this
option, so I won't be repeating myself.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-10-10  2:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-09 18:31 [PATCH 1/2] branch: introduce --current display option Daniels Umanovskis
2018-10-09 18:31 ` [PATCH 2/2] doc/git-branch: Document the --current option Daniels Umanovskis
2018-10-10  2:48   ` Junio C Hamano
2018-10-09 19:54 ` [PATCH 1/2] branch: introduce --current display option Stefan Beller
2018-10-09 20:21   ` Daniels Umanovskis

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).