Daniels Umanovskis <daniels@xxxxxxxxxxxxx> 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.
