On Mon, Feb 27, 2017 at 03:05:37PM -0800, Jacob Keller wrote:

> > Perhaps with s/not_in_refs_heads/not_a_branch_name/ (or swapping
> > polarity, "is_a_branch_name"), the resulting code may not be too
> > hard to read?
> 
> What about changing interpret-branch-name gains a flag to return a
> fully qualified ref rather than returning just the name? That seems
> like it would be more reasonable behavior.

That's not sufficient. If I feed "refs/remotes/origin/master" as a
branch name to git-branch, then as silly as that is, it is the branch
whose ref is "refs/heads/refs/remotes/origin/master".

Since interpret_branch_name() is not fully qualifying everything, but
rather just _sometimes_ replace @-marks with refnames, we cannot tell
from just the string the difference between "the user fed us
refs/remotes/foo" and "the @-mark expanded to a non-branch
refs/remotes/foo". We need one extra bit of information to know whether
an expansion occurred.

You could give a flag that says "do not expand to anything outside of
refs/heads/" that would suppress the @->HEAD mark, as well as
@{upstream} when upstream is outside of refs/heads. That would certainly
be less nasty than the out-parameter, but I wasn't sure that the error
handling was what we wanted.

In strbuf_branchname(), we quietly turn that error into a "0", which
causes us to retain the original text. We then feed that into
check_refname_format() in strbuf_check_branch_ref(). Which I think would
complain, and you'd get "not a valid refname: @{upstream}". If we have
the out-bit, we can say "I understand what @{upstream} means, but it
does not expand to a local branch". That's a more specific error, but
maybe it is not worth the hassle to produce it.

-Peff

Reply via email to