Re: [PATCH] [kernel] completion: silence "fatal: Not a git repository" error

From: John Szakmeister <john@szakmeister.net>
Date: Oct 14, 2014, 19:18 UTC
Message-ID: <CAEBDL5V_Mzxwc4fnybg9=fmeotGV91XerzTccHMWLV79bE+mVA@mail.gmail.com>
In-Reply-To: <xmqqfveqzeqy.fsf@gitster.dls.corp.google.com>

On Tue, Oct 14, 2014 at 2:29 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 23 quoted lines

> John Szakmeister <john@szakmeister.net> writes:
>
>> It is possible that a user is trying to run a git command and fail to realize
>> that they are not in a git repository or working tree.  When trying to complete
>> an operation, __git_refs would fall to a degenerate case and attempt to use
>> "git for-each-ref", which would emit the error.
>>
>> Let's fix this by shunting the error message coming from "git for-each-ref".
>
> Hmph, do you mean this one?
>
>     $ cd /var/tmp ;# not a git repository
>     $ git checkout <TAB>
>
> ->
>
>     $ git checkout fatal: Not a git repository (or any of the parent directories): .git
>     HEAD
>
> I agree it is ugly, but would it be an improvement for the end user,
> who did not realize that she was not in a directory where "git checkout"
> makes sense, not to tell her that she is not in a git repository in
> some way?

I had thought about that too, but I think--for me--it comes down to two things:

1) We're not intentionally trying to inform the user anywhere else
that they are not in a git repo.  We simply fail to complete anything,
which I think is an established behavior.
2) It mingles with the stuff already on the command line, making it
confusing to know what you typed.  Then you end up ctrl-c'ing your way
out of it and starting over--which is the frustrating part.

For me, I thought it better to just be more well-behaved. I've also run across this issue when I legitimately wanted to do something--I wish I could remember what it was--with a remote repo and didn't happen to be in a git working tree. It was frustrating to see this error message then too, for the same reason as above. I use tab completion quite extensively, so spitting things like this out making it difficult to move forward is a problem.

Would it be better to check that "$dir" is non-empty and then provide the extra bits of information? We could then avoid giving the user anything in that case.

-John