[2/2] doc/git-branch: Document the --current option
diff mbox series

Message ID 20181009183114.16477-2-daniels@umanovskis.se
State New
Headers show
  • [1/2] branch: introduce --current display option
Related show

Commit Message

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


Junio C Hamano Oct. 10, 2018, 2:48 a.m. UTC | #1
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

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

diff mbox series

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
 '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).
+	Print the name of the current branch. In detached HEAD state,
+	or if otherwise impossible to resolve the branch name, print
+	"HEAD".