Re: [PATCHv1 2/2] git-p4: work with a detached head

From: Luke Diamand <luke@diamand.org>
Date: Sep 10, 2015, 07:29 UTC
Message-ID: <CAE5ih7-_zhxf_daVxNB-hxFfbakMbJp9rk0vP+k46ErrJ6Qxiw@mail.gmail.com>
In-Reply-To: <xmqqfv2n5khe.fsf@gitster.mtv.corp.google.com>

On 9 September 2015 at 22:52, Junio C Hamano <gitster@pobox.com> wrote:

Show 23 quoted lines

> Luke Diamand <luke@diamand.org> writes:
>
>>      def run(self, args):
>>          if len(args) == 0:
>>              self.master = currentGitBranch()
>> -            if len(self.master) == 0 or not gitBranchExists("refs/heads/%s" % self.master):
>> -                die("Detecting current git branch failed!")
>> +            if self.master == "undefined":
>> +                self.master = None
>
> The comparison with textual "undefined" smelled fishy and I ended up
> looking at the implementation of currentGitBranch().
>
>     def currentGitBranch():
>         return read_pipe("git name-rev HEAD").split(" ")[1].strip()
>
> Yuck.  I know it is not entirely the fault of this patch, but
> shouldn't it be reading from
>
>     $ git symbolic-ref HEAD
>
> and catch the error "fatal: ref HEAD is not a symbolic ref" and use
> it as a signal to tell that the HEAD is detached?

That sounds much nicer. I'll redo the patch accordingly.

Thanks, Luke