{"thread":{"id":"37725","subject":"[PATCH] [kernel] completion: silence \"fatal: Not a git repository\" error","startedAt":"2014-10-14T10:49:45Z","lastAt":"2014-10-14T22:14:06Z","messageCount":5,"participants":["John Szakmeister","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"250602","messageId":"1413283785-505-1-git-send-email-john@szakmeister.net","threadId":"37725","inReplyTo":null,"subject":"[PATCH] [kernel] completion: silence \"fatal: Not a git repository\" error","fromName":"John Szakmeister","fromEmail":"john@szakmeister.net","sentAt":"2014-10-14T10:49:45Z","receivedAt":"2014-10-14T10:49:45Z","isPatch":true,"sender":{"key":"john@szakmeister.net","avatar":"https://avatars.githubusercontent.com/u/448087?v=4"},"body":"It is possible that a user is trying to run a git command and fail to realize\nthat they are not in a git repository or working tree.  When trying to complete\nan operation, __git_refs would fall to a degenerate case and attempt to use\n\"git for-each-ref\", which would emit the error.\n\nLet's fix this by shunting the error message coming from \"git for-each-ref\".\n\nSigned-off-by: John Szakmeister <john@szakmeister.net>\n---\n contrib/completion/git-completion.bash | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 5ea5b82..31b4739 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -388,7 +388,8 @@ __git_refs ()\n \t\t;;\n \t*)\n \t\techo \"HEAD\"\n-\t\tgit for-each-ref --format=\"%(refname:short)\" -- \"refs/remotes/$dir/\" | sed -e \"s#^$dir/##\"\n+\t\tgit for-each-ref --format=\"%(refname:short)\" -- \\\n+\t\t\t\"refs/remotes/$dir/\" 2>/dev/null | sed -e \"s#^$dir/##\"\n \t\t;;\n \tesac\n }\n-- \n2.0.1\n"},{"id":"250609","messageId":"CAEBDL5Uh-LE36EmQ9hzzHKv4+=hjmj5S2z54wMmrpCWZyg9ziw@mail.gmail.com","threadId":"37725","inReplyTo":"1413283785-505-1-git-send-email-john@szakmeister.net","subject":"Re: [PATCH] [kernel] completion: silence \"fatal: Not a git repository\" error","fromName":"John Szakmeister","fromEmail":"john@szakmeister.net","sentAt":"2014-10-14T13:58:08Z","receivedAt":"2014-10-14T13:58:08Z","isPatch":true,"sender":{"key":"john@szakmeister.net","avatar":"https://avatars.githubusercontent.com/u/448087?v=4"},"body":"On Tue, Oct 14, 2014 at 6:49 AM, John Szakmeister <john@szakmeister.net> wrote:\n> It is possible that a user is trying to run a git command and fail to realize\n> that they are not in a git repository or working tree.  When trying to complete\n> an operation, __git_refs would fall to a degenerate case and attempt to use\n> \"git for-each-ref\", which would emit the error.\n>\n> Let's fix this by shunting the error message coming from \"git for-each-ref\".\n>\n> Signed-off-by: John Szakmeister <john@szakmeister.net>\n> ---\n\nSorry for the \"[kernel]\" in the subject.  I must have forgotten to\nremove that off of my format-patch invocation.  If you need me to\nresubmit without it, I can do that.\n\nThanks!\n\n-John\n"},{"id":"250619","messageId":"xmqqfveqzeqy.fsf@gitster.dls.corp.google.com","threadId":"37725","inReplyTo":"1413283785-505-1-git-send-email-john@szakmeister.net","subject":"Re: [PATCH] [kernel] completion: silence \"fatal: Not a git repository\" error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-14T18:29:41Z","receivedAt":"2014-10-14T18:29:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Szakmeister <john@szakmeister.net> writes:\n\n> It is possible that a user is trying to run a git command and fail to realize\n> that they are not in a git repository or working tree.  When trying to complete\n> an operation, __git_refs would fall to a degenerate case and attempt to use\n> \"git for-each-ref\", which would emit the error.\n>\n> Let's fix this by shunting the error message coming from \"git for-each-ref\".\n\nHmph, do you mean this one?\n\n    $ cd /var/tmp ;# not a git repository\n    $ git checkout <TAB>\n\n->\n\n    $ git checkout fatal: Not a git repository (or any of the parent directories): .git\n    HEAD \n\nI agree it is ugly, but would it be an improvement for the end user,\nwho did not realize that she was not in a directory where \"git checkout\"\nmakes sense, not to tell her that she is not in a git repository in\nsome way?\n"},{"id":"250623","messageId":"CAEBDL5V_Mzxwc4fnybg9=fmeotGV91XerzTccHMWLV79bE+mVA@mail.gmail.com","threadId":"37725","inReplyTo":"xmqqfveqzeqy.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] [kernel] completion: silence \"fatal: Not a git repository\" error","fromName":"John Szakmeister","fromEmail":"john@szakmeister.net","sentAt":"2014-10-14T19:18:17Z","receivedAt":"2014-10-14T19:18:17Z","isPatch":true,"sender":{"key":"john@szakmeister.net","avatar":"https://avatars.githubusercontent.com/u/448087?v=4"},"body":"On Tue, Oct 14, 2014 at 2:29 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> John Szakmeister <john@szakmeister.net> writes:\n>\n>> It is possible that a user is trying to run a git command and fail to realize\n>> that they are not in a git repository or working tree.  When trying to complete\n>> an operation, __git_refs would fall to a degenerate case and attempt to use\n>> \"git for-each-ref\", which would emit the error.\n>>\n>> Let's fix this by shunting the error message coming from \"git for-each-ref\".\n>\n> Hmph, do you mean this one?\n>\n>     $ cd /var/tmp ;# not a git repository\n>     $ git checkout <TAB>\n>\n> ->\n>\n>     $ git checkout fatal: Not a git repository (or any of the parent directories): .git\n>     HEAD\n>\n> I agree it is ugly, but would it be an improvement for the end user,\n> who did not realize that she was not in a directory where \"git checkout\"\n> makes sense, not to tell her that she is not in a git repository in\n> some way?\n\nI had thought about that too, but I think--for me--it comes down to two things:\n\n1) We're not intentionally trying to inform the user anywhere else\nthat they are not in a git repo.  We simply fail to complete anything,\nwhich I think is an established behavior.\n2) It mingles with the stuff already on the command line, making it\nconfusing to know what you typed.  Then you end up ctrl-c'ing your way\nout of it and starting over--which is the frustrating part.\n\nFor me, I thought it better to just be more well-behaved.  I've also\nrun across this issue when I legitimately wanted to do something--I\nwish I could remember what it was--with a remote repo and didn't\nhappen to be in a git working tree.  It was frustrating to see this\nerror message then too, for the same reason as above.  I use tab\ncompletion quite extensively, so spitting things like this out making\nit difficult to move forward is a problem.\n\nWould it be better to check that \"$dir\" is non-empty and then provide\nthe extra bits of information?  We could then avoid giving the user\nanything in that case.\n\n-John\n"},{"id":"250628","messageId":"xmqqiojmxpsh.fsf@gitster.dls.corp.google.com","threadId":"37725","inReplyTo":"CAEBDL5V_Mzxwc4fnybg9=fmeotGV91XerzTccHMWLV79bE+mVA@mail.gmail.com","subject":"Re: [PATCH] [kernel] completion: silence \"fatal: Not a git repository\" error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-14T22:14:06Z","receivedAt":"2014-10-14T22:14:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Szakmeister <john@szakmeister.net> writes:\n\n> On Tue, Oct 14, 2014 at 2:29 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> ...\n>> Hmph, do you mean this one?\n>>\n>>     $ cd /var/tmp ;# not a git repository\n>>     $ git checkout <TAB>\n>>\n>> ->\n>>\n>>     $ git checkout fatal: Not a git repository (or any of the parent directories): .git\n>>     HEAD\n>>\n>> I agree it is ugly, but would it be an improvement for the end user,\n>> who did not realize that she was not in a directory where \"git checkout\"\n>> makes sense, not to tell her that she is not in a git repository in\n>> some way?\n>\n> I had thought about that too, but I think--for me--it comes down to two things:\n>\n> 1) We're not intentionally trying to inform the user anywhere else\n> that they are not in a git repo.  We simply fail to complete anything,\n> which I think is an established behavior.\n> 2) It mingles with the stuff already on the command line, making it\n> confusing to know what you typed.  Then you end up ctrl-c'ing your way\n> out of it and starting over--which is the frustrating part.\n\nIt is not that I am unsympathetic.  It's just it looks to me that\nthe patch is potentially adding one more failed step by hiding the\nerror message to further frustrate the user.\n\n    $ git checkout <TAB>\n    ... completes nothing; puzzled but decides not to be worried for now\n    $ git checkout master<RET>\n    fatal: not a git repository\n\nAs you noticed, however, we do not show the ugly error message by\ndesign.  It is not done consistently, either (happens only when we\ntry to complete refnames).\n\nI was just hoping that somebody (not necessarily you) could suggest\na way to do better than hide the error message only because it looks\nugly (iow, perhaps show it not in the middle of the command line,\nand do so more consistently).  Yes I would imagine it would be a lot\nharder, but the end user experience _might_ become so much better to\nmake it worthwhile.  I dunno.\n\nI am not strongly opposed to queuing the patch.\n"}]}