Re: [Question] builtin/branch.c

From: Jeff King <peff@peff.net>
Date: Oct 15, 2018, 17:14 UTC
Message-ID: <20181015171417.GA1301@sigill.intra.peff.net>
In-Reply-To: <CACBZZX7i-Uob9EJ8GeDKYVTyKtdRiy=qPbxSOe=FGh2cbHG9Zw@mail.gmail.com>

On Sun, Oct 14, 2018 at 12:19:35PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 18 quoted lines

> On Sat, Oct 13, 2018 at 10:12 AM Tao Qingyun <taoqy@ls-a.me> wrote:
> > Hi, I am learning `builtin/branch.c`. I find that it will call `branch_get`
> > before create and [un]set upstream, and die with "no such branch" if failed.
> > but `branch_get` seems never fail, it is a get_or_create. Also, it was
> > confused that getting a branch before it has created.
> >
> > builtin/branch.c #811
> >
> >     } else if (argc > 0 && argc <= 2) {
> >         struct branch *branch = branch_get(argv[0]);
> >
> >         if (!branch)
> >             die(_("no such branch '%s'"), argv[0]);
> 
> From my reading of the source you're correct. That !branch case is
> pointless. The only way that function can fail is in the x*() family
> of functions, which'll make the function die instead of returning
> NULL.

It sometimes returns current_branch, which can be NULL (e.g., if you're on a detached HEAD). Try:

  $ git branch HEAD
  fatal: no such branch 'HEAD'

  $ git branch ''
  fatal: no such branch ''

However, it seems weird that we'd check those cases here (and provide such lousy messages). And indeed, dropping that and letting us eventually hit create_branch() gives a much better message:

  $ git branch HEAD
  fatal: 'HEAD' is not a valid branch name.

  $ git branch ''
  fatal: '' is not a valid branch name.

I think we'd want to see that reasoning in the commit message.

-Peff