{"thread":{"id":"41329","subject":"[PATCH] git-completion.bash: always swallow error output of for-each-ref","startedAt":"2016-02-04T10:34:59Z","lastAt":"2016-02-24T07:48:52Z","messageCount":24,"participants":["Sebastian Schuberth","Johannes Schindelin","Junio C Hamano","Jeff King","SZEDER Gábor","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"277417","messageId":"56B32953.2010908@gmail.com","threadId":"41329","inReplyTo":null,"subject":"[PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2016-02-04T10:34:59Z","receivedAt":"2016-02-04T10:34:59Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"This avoids output like\n\n    warning: ignoring broken ref refs/remotes/origin/HEAD\n\nwhile completing branch names.\n\nSigned-off-by: Sebastian Schuberth <sschuberth@gmail.com>\n---\n contrib/completion/git-completion.bash | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 15ebba5..7c0549d 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -317,7 +317,7 @@ __git_heads ()\n \tlocal dir=\"$(__gitdir)\"\n \tif [ -d \"$dir\" ]; then\n \t\tgit --git-dir=\"$dir\" for-each-ref --format='%(refname:short)' \\\n-\t\t\trefs/heads\n+\t\t\trefs/heads 2>/dev/null\n \t\treturn\n \tfi\n }\n@@ -327,7 +327,7 @@ __git_tags ()\n \tlocal dir=\"$(__gitdir)\"\n \tif [ -d \"$dir\" ]; then\n \t\tgit --git-dir=\"$dir\" for-each-ref --format='%(refname:short)' \\\n-\t\t\trefs/tags\n+\t\t\trefs/tags 2>/dev/null\n \t\treturn\n \tfi\n }\n@@ -355,14 +355,14 @@ __git_refs ()\n \t\t\t;;\n \t\tesac\n \t\tgit --git-dir=\"$dir\" for-each-ref --format=\"%($format)\" \\\n-\t\t\t$refs\n+\t\t\t$refs 2>/dev/null\n \t\tif [ -n \"$track\" ]; then\n \t\t\t# employ the heuristic used by git checkout\n \t\t\t# Try to find a remote branch that matches the completion word\n \t\t\t# but only output if the branch name is unique\n \t\t\tlocal ref entry\n \t\t\tgit --git-dir=\"$dir\" for-each-ref --shell --format=\"ref=%(refname:short)\" \\\n-\t\t\t\t\"refs/remotes/\" | \\\n+\t\t\t\t\"refs/remotes/\" 2>/dev/null | \\\n \t\t\twhile read -r entry; do\n \t\t\t\teval \"$entry\"\n \t\t\t\tref=\"${ref#*/}\"\n@@ -1835,7 +1835,7 @@ _git_config ()\n \t\tremote=\"${remote%.push}\"\n \t\t__gitcomp_nl \"$(git --git-dir=\"$(__gitdir)\" \\\n \t\t\tfor-each-ref --format='%(refname):%(refname)' \\\n-\t\t\trefs/heads)\"\n+\t\t\trefs/heads 2>/dev/null)\"\n \t\treturn\n \t\t;;\n \tpull.twohead|pull.octopus)\n-- \n2.7.0.windows.1\n"},{"id":"282595","messageId":"20160204111307.GA30495@sigill.intra.peff.net","threadId":"41329","inReplyTo":"56B32953.2010908@gmail.com","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-04T11:13:07Z","receivedAt":"2016-02-04T11:13:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 04, 2016 at 11:34:59AM +0100, Sebastian Schuberth wrote:\n\n> This avoids output like\n> \n>     warning: ignoring broken ref refs/remotes/origin/HEAD\n> \n> while completing branch names.\n\nHmm. I feel like this case (HEAD points to a branch, then `fetch\n--prune` deletes it) came up recently and we discussed quieting that\nwarning. But now I cannot seem to find it.\n\nAnyway, I this is a reasonable workaround. Errors from bash completion\nscripts are almost always going to be useless and get in the way of\nreading your own prompt.\n\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 15ebba5..7c0549d 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -317,7 +317,7 @@ __git_heads ()\n>  \tlocal dir=\"$(__gitdir)\"\n>  \tif [ -d \"$dir\" ]; then\n>  \t\tgit --git-dir=\"$dir\" for-each-ref --format='%(refname:short)' \\\n> -\t\t\trefs/heads\n> +\t\t\trefs/heads 2>/dev/null\n>  \t\treturn\n\nNot really related to your topic, but digging into it caused me to read\nb7dd2d2 (for-each-ref: Do not lookup objects when they will not be used,\n2009-05-27), which is about making sure for-each-ref is very fast in\ncompletion.\n\nIt looks like %(refname:short) is actually kind of expensive:\n\n$ time git for-each-ref --format='%(refname)' refs/tags  >/dev/null\n\nreal    0m0.004s\nuser    0m0.000s\nsys     0m0.004s\n\n$ time git for-each-ref --format='%(refname:short)' refs/tags >/dev/null\n\nreal    0m0.009s\nuser    0m0.004s\nsys     0m0.004s\n\nThe upcoming refname:strip does much better:\n\n$ time git for-each-ref --format='%(refname:strip=2)' refs/tags >/dev/null\n\nreal    0m0.004s\nuser    0m0.000s\nsys     0m0.004s\n\nObviously these are pretty small timings from my git.git with ~600 tags,\nbut you can see that refname:short really does cost more.  On a more\nridiculous example repository I have with about 10 million refs, the\ntimings are more like 5s, 66s, 5.5s.\n\nJust thought I'd throw that our there in case any completion people feel\nlike poking around with it.\n\n-Peff\n"},{"id":"277419","messageId":"alpine.DEB.2.20.1602041216240.2964@virtualbox","threadId":"41329","inReplyTo":"20160204111307.GA30495@sigill.intra.peff.net","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-02-04T11:26:19Z","receivedAt":"2016-02-04T11:26:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Thu, 4 Feb 2016, Jeff King wrote:\n\n> On Thu, Feb 04, 2016 at 11:34:59AM +0100, Sebastian Schuberth wrote:\n> \n> > This avoids output like\n> > \n> >     warning: ignoring broken ref refs/remotes/origin/HEAD\n> > \n> > while completing branch names.\n> \n> Hmm. I feel like this case (HEAD points to a branch, then `fetch\n> --prune` deletes it) came up recently and we discussed quieting that\n> warning. But now I cannot seem to find it.\n\nI am pretty certain that it came up in my patch series:\n\n\thttp://thread.gmane.org/gmane.comp.version-control.git/278538\n\n> Anyway, I this is a reasonable workaround. Errors from bash completion\n> scripts are almost always going to be useless and get in the way of\n> reading your own prompt.\n\nMaybe we should just shut up the completions in more cases? Dunno...\n\n> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> > index 15ebba5..7c0549d 100644\n> > --- a/contrib/completion/git-completion.bash\n> > +++ b/contrib/completion/git-completion.bash\n> > @@ -317,7 +317,7 @@ __git_heads ()\n> >  \tlocal dir=\"$(__gitdir)\"\n> >  \tif [ -d \"$dir\" ]; then\n> >  \t\tgit --git-dir=\"$dir\" for-each-ref --format='%(refname:short)' \\\n> > -\t\t\trefs/heads\n> > +\t\t\trefs/heads 2>/dev/null\n> >  \t\treturn\n> \n> Not really related to your topic, but digging into it caused me to read\n> b7dd2d2 (for-each-ref: Do not lookup objects when they will not be used,\n> 2009-05-27), which is about making sure for-each-ref is very fast in\n> completion.\n> \n> It looks like %(refname:short) is actually kind of expensive:\n\nYep, this was reported on the Git for Windows bug tracker, too:\n\n\thttps://github.com/git-for-windows/git/issues/524\n\n> $ time git for-each-ref --format='%(refname)' refs/tags  >/dev/null\n> \n> real    0m0.004s\n> user    0m0.000s\n> sys     0m0.004s\n> \n> $ time git for-each-ref --format='%(refname:short)' refs/tags >/dev/null\n> \n> real    0m0.009s\n> user    0m0.004s\n> sys     0m0.004s\n\nAnd the timings in the ticket I mentioned above are not pretty small:\n0.055s vs 1.341s\n\n> The upcoming refname:strip does much better:\n> \n> $ time git for-each-ref --format='%(refname:strip=2)' refs/tags >/dev/null\n> \n> real    0m0.004s\n> user    0m0.000s\n> sys     0m0.004s\n\nThis is funny: after reading the commit message at\nhttps://github.com/git/git/commit/0571979b it eludes me why strip=2 should\nbe so much faster than short...\n\nCiao,\nDscho\n"},{"id":"282597","messageId":"20160204114506.GA1710@sigill.intra.peff.net","threadId":"41329","inReplyTo":"alpine.DEB.2.20.1602041216240.2964@virtualbox","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-04T11:45:06Z","receivedAt":"2016-02-04T11:45:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 04, 2016 at 12:26:19PM +0100, Johannes Schindelin wrote:\n\n> > Hmm. I feel like this case (HEAD points to a branch, then `fetch\n> > --prune` deletes it) came up recently and we discussed quieting that\n> > warning. But now I cannot seem to find it.\n> \n> I am pretty certain that it came up in my patch series:\n> \n> \thttp://thread.gmane.org/gmane.comp.version-control.git/278538\n\nGood, I'm not going crazy! But my search skills are apparently\natrophying. :)\n\nIt looks like we just addressed the git-gc issue there. for-each-ref\nuses the \"rawref\" interface, so it gets fed broken things and warns\nabout them.\n\nI'm tempted to say that it should just silently ignore broken symrefs,\nas they're kind-of a normal thing. But I also think Sebastian's patch to\nsquelch stderr during completion is quite reasonable, too.\n\n> This is funny: after reading the commit message at\n> https://github.com/git/git/commit/0571979b it eludes me why strip=2 should\n> be so much faster than short...\n\n:short is slow because it checks for ambiguity. So it has to walk the\ndwim_ref() rules backwards, checking if each possibility is an existing\nref.\n\nWhereas strip=2 is literally just skipping past the early bits of the\nrefname string.\n\n-Peff\n"},{"id":"277445","messageId":"xmqqwpqki9bh.fsf@gitster.mtv.corp.google.com","threadId":"41329","inReplyTo":"alpine.DEB.2.20.1602041216240.2964@virtualbox","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-04T19:06:58Z","receivedAt":"2016-02-04T19:06:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> $ time git for-each-ref --format='%(refname:short)' refs/tags >/dev/null\n>> \n>> real    0m0.009s\n>> user    0m0.004s\n>> sys     0m0.004s\n>\n> And the timings in the ticket I mentioned above are not pretty small:\n> 0.055s vs 1.341s\n>\n>> The upcoming refname:strip does much better:\n>> \n>> $ time git for-each-ref --format='%(refname:strip=2)' refs/tags >/dev/null\n>> \n>> real    0m0.004s\n>> user    0m0.000s\n>> sys     0m0.004s\n>\n> This is funny: after reading the commit message at\n> https://github.com/git/git/commit/0571979b it eludes me why strip=2 should\n> be so much faster than short...\n\n\"short\" tries to ensure that the result is not ambiguous within the\nrepository, so when asked to shorten refs/heads/foo, it needs to\ncheck if refs/tags/foo exists.  \"strip=2\" textually strips two\nlevels from the top without worrying about ambiguity across\ndifferent hierarchies.\n"},{"id":"278001","messageId":"56BDA48F.6020305@gmail.com","threadId":"41329","inReplyTo":"56B32953.2010908@gmail.com","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2016-02-12T09:23:27Z","receivedAt":"2016-02-12T09:23:27Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On 04.02.2016 11:34, Sebastian Schuberth wrote:\n\n> This avoids output like\n>\n>      warning: ignoring broken ref refs/remotes/origin/HEAD\n>\n> while completing branch names.\n>\n> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>\n\nThe discussion got a bit off the point with the \"short\" vs. \"strip=2\" \nstuff, but I guess the patch itself if good to apply?\n\n-- \nSebastian Schuberth\n"},{"id":"278033","messageId":"xmqqsi0xu2ac.fsf@gitster.mtv.corp.google.com","threadId":"41329","inReplyTo":"20160204111307.GA30495@sigill.intra.peff.net","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-12T20:00:43Z","receivedAt":"2016-02-12T20:00:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Feb 04, 2016 at 11:34:59AM +0100, Sebastian Schuberth wrote:\n>\n>> This avoids output like\n>> \n>>     warning: ignoring broken ref refs/remotes/origin/HEAD\n>> \n>> while completing branch names.\n>\n> Hmm. I feel like this case (HEAD points to a branch, then `fetch\n> --prune` deletes it) came up recently and we discussed quieting that\n> warning. But now I cannot seem to find it.\n>\n> Anyway, I this is a reasonable workaround. Errors from bash completion\n> scripts are almost always going to be useless and get in the way of\n> reading your own prompt.\n\nI think that is absolutely the right stance to take, but then I\nwonder if it is a sensible execution to sprinkle 2>/dev/null\neverywhere.\n\nFor example, couldn't we do something like this instead?\n\nThis is just for illustration and does not remove all 2>/dev/null\nand replace them with a single redirection that covers the entire\nshell function body, but something along this line smells a lot more\npleasant.  I dunno.\n\n contrib/completion/git-completion.bash | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex ba4137d..637c42d 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -47,14 +47,14 @@ __gitdir ()\n \t\telif [ -d .git ]; then\n \t\t\techo .git\n \t\telse\n-\t\t\tgit rev-parse --git-dir 2>/dev/null\n+\t\t\tgit rev-parse --git-dir\n \t\tfi\n \telif [ -d \"$1/.git\" ]; then\n \t\techo \"$1/.git\"\n \telse\n \t\techo \"$1\"\n \tfi\n-}\n+} 2>/dev/null\n \n # The following function is based on code from:\n #\n@@ -320,7 +320,7 @@ __git_heads ()\n \t\t\trefs/heads\n \t\treturn\n \tfi\n-}\n+} 2>/dev/null\n \n __git_tags ()\n {\n@@ -330,7 +330,7 @@ __git_tags ()\n \t\t\trefs/tags\n \t\treturn\n \tfi\n-}\n+} 2>/dev/null\n \n # __git_refs accepts 0, 1 (to pass to __gitdir), or 2 arguments\n # presence of 2nd argument means use the guess heuristic employed\n@@ -389,7 +389,7 @@ __git_refs ()\n \t\t\t\"refs/remotes/$dir/\" 2>/dev/null | sed -e \"s#^$dir/##\"\n \t\t;;\n \tesac\n-}\n+} 2>/dev/null\n \n # __git_refs2 requires 1 argument (to pass to __git_refs)\n __git_refs2 ()\n"},{"id":"278034","messageId":"20160212201002.GA21598@sigill.intra.peff.net","threadId":"41329","inReplyTo":"xmqqsi0xu2ac.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-12T20:10:02Z","receivedAt":"2016-02-12T20:10:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 12, 2016 at 12:00:43PM -0800, Junio C Hamano wrote:\n\n> > Anyway, I this is a reasonable workaround. Errors from bash completion\n> > scripts are almost always going to be useless and get in the way of\n> > reading your own prompt.\n> \n> I think that is absolutely the right stance to take, but then I\n> wonder if it is a sensible execution to sprinkle 2>/dev/null\n> everywhere.\n> \n> For example, couldn't we do something like this instead?\n> \n> This is just for illustration and does not remove all 2>/dev/null\n> and replace them with a single redirection that covers the entire\n> shell function body, but something along this line smells a lot more\n> pleasant.  I dunno.\n\nI agree it's a lot more pleasant, assuming there are no cases where we\nwould want to pass through an error. But I really cannot think of one.\nEven explosive \"woah, your git repo is totally corrupted\" messages\nprobably should be suppressed in the prompt.\n\n> @@ -320,7 +320,7 @@ __git_heads ()\n>  \t\t\trefs/heads\n>  \t\treturn\n>  \tfi\n> -}\n> +} 2>/dev/null\n\nToday I learned about yet another fun corner of POSIX shell.\n\n-Peff\n"},{"id":"278035","messageId":"xmqqoablu13j.fsf@gitster.mtv.corp.google.com","threadId":"41329","inReplyTo":"20160212201002.GA21598@sigill.intra.peff.net","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-12T20:26:24Z","receivedAt":"2016-02-12T20:26:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I agree it's a lot more pleasant, assuming there are no cases where we\n> would want to pass through an error. But I really cannot think of one.\n> Even explosive \"woah, your git repo is totally corrupted\" messages\n> probably should be suppressed in the prompt.\n>\n>> @@ -320,7 +320,7 @@ __git_heads ()\n>>  \t\t\trefs/heads\n>>  \t\treturn\n>>  \tfi\n>> -}\n>> +} 2>/dev/null\n>\n> Today I learned about yet another fun corner of POSIX shell.\n\nI may have learned about this soon after I started learning bash,\nbut I admit that this was the first time I found a practical use\ncase for it ;-).\n"},{"id":"278036","messageId":"20160212224048.Horde.IpOeDKLAMM4a11F2xyIeY4M@webmail.informatik.kit.edu","threadId":"41329","inReplyTo":"xmqqsi0xu2ac.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2016-02-12T21:40:48Z","receivedAt":"2016-02-12T21:40:48Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Junio C Hamano <gitster@pobox.com>:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> On Thu, Feb 04, 2016 at 11:34:59AM +0100, Sebastian Schuberth wrote:\n>>\n>>> This avoids output like\n>>>\n>>>     warning: ignoring broken ref refs/remotes/origin/HEAD\n>>>\n>>> while completing branch names.\n>>\n>> Hmm. I feel like this case (HEAD points to a branch, then `fetch\n>> --prune` deletes it) came up recently and we discussed quieting that\n>> warning. But now I cannot seem to find it.\n>>\n>> Anyway, I this is a reasonable workaround. Errors from bash completion\n>> scripts are almost always going to be useless and get in the way of\n>> reading your own prompt.\n>\n> I think that is absolutely the right stance to take, but then I\n> wonder if it is a sensible execution to sprinkle 2>/dev/null\n> everywhere.\n>\n> For example, couldn't we do something like this instead?\n>\n> This is just for illustration and does not remove all 2>/dev/null\n> and replace them with a single redirection that covers the entire\n> shell function body, but something along this line smells a lot more\n> pleasant.  I dunno.\n\nPlease no :)\n\nFirst, we don't have to redirect stderr of every completion function,\nit's sufficient to do so only for the two \"main\" entry point functions\n__git_main() and __gitk_main().\n\nBut:\n\n  * It would swallow even those errors that we are interested in,\n    e.g. (note the missing quotes around $foo):\n\n       $ func () { if [ $foo = y ] ; then echo \"foo is y\" ; fi ; }\n       $ foo=\n       $ func 2>/dev/null\n       $ func\n       bash: [: =: unary operator expected\n\n    Something like this should not happen, it's a bug in the\n    completion script that should be fixed, and we should get a bug\n    report.\n\n  * I often find myself tracing/debugging the completion script\n    through stderr by scattering\n\n       echo >&2 \"foo: '$foo'\"\n\n    and the like all over the place.  If completion functions' stderr\n    were redirected, then I would have to disable that redirection\n    first to be able do this kind of poor man's tracing.\n\n  * I have a WIP patch series that deals with errors from git\n    commands.\n    It's a mixed bag of __gitdir()-related cleanups, fixes and\n    optimizations, which factors out all git executions into a\n    __git() wrapper function and redirects stderr only in that\n    function, thereby eliminating most of the 2>/dev/null\n    redirections in the completion script.\n    It still needs some work to iron out a wrinkle or two around\n    corner cases, though.\n\n\n\n>  contrib/completion/git-completion.bash | 10 +++++-----\n>  1 file changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/contrib/completion/git-completion.bash  \n> b/contrib/completion/git-completion.bash\n> index ba4137d..637c42d 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -47,14 +47,14 @@ __gitdir ()\n>  \t\telif [ -d .git ]; then\n>  \t\t\techo .git\n>  \t\telse\n> -\t\t\tgit rev-parse --git-dir 2>/dev/null\n> +\t\t\tgit rev-parse --git-dir\n>  \t\tfi\n>  \telif [ -d \"$1/.git\" ]; then\n>  \t\techo \"$1/.git\"\n>  \telse\n>  \t\techo \"$1\"\n>  \tfi\n> -}\n> +} 2>/dev/null\n>\n>  # The following function is based on code from:\n>  #\n> @@ -320,7 +320,7 @@ __git_heads ()\n>  \t\t\trefs/heads\n>  \t\treturn\n>  \tfi\n> -}\n> +} 2>/dev/null\n>\n>  __git_tags ()\n>  {\n> @@ -330,7 +330,7 @@ __git_tags ()\n>  \t\t\trefs/tags\n>  \t\treturn\n>  \tfi\n> -}\n> +} 2>/dev/null\n>\n>  # __git_refs accepts 0, 1 (to pass to __gitdir), or 2 arguments\n>  # presence of 2nd argument means use the guess heuristic employed\n> @@ -389,7 +389,7 @@ __git_refs ()\n>  \t\t\t\"refs/remotes/$dir/\" 2>/dev/null | sed -e \"s#^$dir/##\"\n>  \t\t;;\n>  \tesac\n> -}\n> +} 2>/dev/null\n>\n>  # __git_refs2 requires 1 argument (to pass to __git_refs)\n>  __git_refs2 ()\n"},{"id":"278037","messageId":"20160212221639.GA27974@sigill.intra.peff.net","threadId":"41329","inReplyTo":"20160212224048.Horde.IpOeDKLAMM4a11F2xyIeY4M@webmail.informatik.kit.edu","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-12T22:16:39Z","receivedAt":"2016-02-12T22:16:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 12, 2016 at 10:40:48PM +0100, SZEDER Gábor wrote:\n\n>  * It would swallow even those errors that we are interested in,\n>    e.g. (note the missing quotes around $foo):\n> [...]\n>  * I often find myself tracing/debugging the completion script\n>    through stderr by scattering\n> \n>       echo >&2 \"foo: '$foo'\"\n\nOne alternative to deal with these would be to add a flag to\nconditionally turn off stderr, and then leave it on during normal\noperation and disable it (letting everything through, including whatever\nrandom cruft git commands produce) for debugging.\n\nBut...\n\n>  * I have a WIP patch series that deals with errors from git\n>    commands.\n\nI'm happy to wait and see what this patch looks like (and generally\nhappy to defer to you on maintenance issues for completion, as you are\nmuch more likely than me to be the one fixing things later on :) ).\n\n-Peff\n"},{"id":"278040","messageId":"20160213002122.Horde.mxoPmZIuCikpV2PO97l11AI@webmail.informatik.kit.edu","threadId":"41329","inReplyTo":"alpine.DEB.2.20.1602041216240.2964@virtualbox","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2016-02-12T23:21:22Z","receivedAt":"2016-02-12T23:21:22Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Johannes Schindelin <Johannes.Schindelin@gmx.de>:\n\n> Hi Peff,\n>\n> On Thu, 4 Feb 2016, Jeff King wrote:\n\n>> > diff --git a/contrib/completion/git-completion.bash  \n>> b/contrib/completion/git-completion.bash\n>> > index 15ebba5..7c0549d 100644\n>> > --- a/contrib/completion/git-completion.bash\n>> > +++ b/contrib/completion/git-completion.bash\n>> > @@ -317,7 +317,7 @@ __git_heads ()\n>> >  \tlocal dir=\"$(__gitdir)\"\n>> >  \tif [ -d \"$dir\" ]; then\n>> >  \t\tgit --git-dir=\"$dir\" for-each-ref --format='%(refname:short)' \\\n>> > -\t\t\trefs/heads\n>> > +\t\t\trefs/heads 2>/dev/null\n>> >  \t\treturn\n>>\n>> Not really related to your topic, but digging into it caused me to read\n>> b7dd2d2 (for-each-ref: Do not lookup objects when they will not be used,\n>> 2009-05-27), which is about making sure for-each-ref is very fast in\n>> completion.\n>>\n>> It looks like %(refname:short) is actually kind of expensive:\n>\n> Yep, this was reported on the Git for Windows bug tracker, too:\n>\n> \thttps://github.com/git-for-windows/git/issues/524\n>\n>> $ time git for-each-ref --format='%(refname)' refs/tags  >/dev/null\n>>\n>> real    0m0.004s\n>> user    0m0.000s\n>> sys     0m0.004s\n>>\n>> $ time git for-each-ref --format='%(refname:short)' refs/tags >/dev/null\n>>\n>> real    0m0.009s\n>> user    0m0.004s\n>> sys     0m0.004s\n>\n> And the timings in the ticket I mentioned above are not pretty small:\n> 0.055s vs 1.341s\n\nIt's ironic that 'refname:short' came about because it was faster than\n'refname' plus removing 'refs/{heads,tags,remotes}/' in a shell loop, at\nleast on Linux.\n\nHowever, 'refname:short' performs a lot more stat() calls per ref to check\nambiguity, especially since many ref races got fixed.  In a repo with a\nsingle master branch:\n\n   $ strace git for-each-ref --format='%(refname)' refs/heads/master  \n2>&1 |grep 'stat(\"\\.git.*\\(master\\|packed-refs\\)'\n   stat(\".git/refs/heads/master\", {st_mode=S_IFREG|0644, st_size=41, ...}) = 0\n   lstat(\".git/refs/heads/master\", {st_mode=S_IFREG|0644, st_size=41, ...}) = 0\n\n   $ strace git for-each-ref --format='%(refname:short)'  \nrefs/heads/master 2>&1 |grep 'stat(\"\\.git.*\\(master\\|packed-refs\\)'\n   stat(\".git/refs/heads/master\", {st_mode=S_IFREG|0644, st_size=41, ...}) = 0\n   lstat(\".git/refs/heads/master\", {st_mode=S_IFREG|0644, st_size=41, ...}) = 0\n   lstat(\".git/master\", 0x7fff6dac9610)    = -1 ENOENT (No such file  \nor directory)\n   stat(\".git/packed-refs\", 0x7fff6dac9460) = -1 ENOENT (No such file  \nor directory)\n   lstat(\".git/refs/master\", 0x7fff6dac9610) = -1 ENOENT (No such file  \nor directory)\n   stat(\".git/packed-refs\", 0x7fff6dac9460) = -1 ENOENT (No such file  \nor directory)\n   lstat(\".git/refs/tags/master\", 0x7fff6dac9610) = -1 ENOENT (No such  \nfile or directory)\n   stat(\".git/packed-refs\", 0x7fff6dac9460) = -1 ENOENT (No such file  \nor directory)\n   lstat(\".git/refs/remotes/master\", 0x7fff6dac9610) = -1 ENOENT (No  \nsuch file or directory)\n   stat(\".git/packed-refs\", 0x7fff6dac9460) = -1 ENOENT (No such file  \nor directory)\n   lstat(\".git/refs/remotes/master/HEAD\", 0x7fff6dac9610) = -1 ENOENT  \n(No such file or directory)\n   stat(\".git/packed-refs\", 0x7fff6dac9460) = -1 ENOENT (No such file  \nor directory)\n\nSince stat()s were never a strong side of Windows, I'm afraid 'refname:short'\nfired backwards and made things much slower over there.  Ouch.\n\nI think in this case we should opt for performance instead of correctness,\nand use Peff's 'refname:strip=2'.  Ambiguous refs will only hurt you, if,\nwell, your repo actually has ambiguous refs AND you happen to want to do\nsomething with one of those refs.  I suspect that's rather uncommon, and\neven then you could simply rename one of those refs.  OTOH, as shown in\nthe ticket, you don't need that many refs to make refs completion\nunacceptably slow on Windows, and it will bite every time you attempt to\ncomplete a ref.\n\nNow, if 'git for-each-ref' could understand '**' globbing, not just\nfnmatch...\n\n\n>> The upcoming refname:strip does much better:\n>>\n>> $ time git for-each-ref --format='%(refname:strip=2)' refs/tags >/dev/null\n>>\n>> real    0m0.004s\n>> user    0m0.000s\n>> sys     0m0.004s\n>\n> This is funny: after reading the commit message at\n> https://github.com/git/git/commit/0571979b it eludes me why strip=2 should\n> be so much faster than short...\n>\n> Ciao,\n> Dscho\n"},{"id":"278049","messageId":"xmqqk2m9ts91.fsf@gitster.mtv.corp.google.com","threadId":"41329","inReplyTo":"20160212221639.GA27974@sigill.intra.peff.net","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-12T23:37:30Z","receivedAt":"2016-02-12T23:37:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, Feb 12, 2016 at 10:40:48PM +0100, SZEDER Gábor wrote:\n>\n>>  * It would swallow even those errors that we are interested in,\n>>    e.g. (note the missing quotes around $foo):\n>> [...]\n>>  * I often find myself tracing/debugging the completion script\n>>    through stderr by scattering\n>> \n>>       echo >&2 \"foo: '$foo'\"\n>\n> One alternative to deal with these would be to add a flag to\n> conditionally turn off stderr, and then leave it on during normal\n> operation and disable it (letting everything through, including whatever\n> random cruft git commands produce) for debugging.\n>\n> But...\n>\n>>  * I have a WIP patch series that deals with errors from git\n>>    commands.\n>\n> I'm happy to wait and see what this patch looks like (and generally\n> happy to defer to you on maintenance issues for completion, as you are\n> much more likely than me to be the one fixing things later on :) ).\n>\n> -Peff\n\nLikewise on both counts.\n"},{"id":"278053","messageId":"20160212234041.GA15688@sigill.intra.peff.net","threadId":"41329","inReplyTo":"20160213002122.Horde.mxoPmZIuCikpV2PO97l11AI@webmail.informatik.kit.edu","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-12T23:40:42Z","receivedAt":"2016-02-12T23:40:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 13, 2016 at 12:21:22AM +0100, SZEDER Gábor wrote:\n\n> I think in this case we should opt for performance instead of correctness,\n> and use Peff's 'refname:strip=2'.  Ambiguous refs will only hurt you, if,\n> well, your repo actually has ambiguous refs AND you happen to want to do\n> something with one of those refs.  I suspect that's rather uncommon, and\n> even then you could simply rename one of those refs.  OTOH, as shown in\n> the ticket, you don't need that many refs to make refs completion\n> unacceptably slow on Windows, and it will bite every time you attempt to\n> complete a ref.\n\nI'm not even sure that this is a correctness tradeoff at all. For\nexample, in the function __git_heads(), we are asking for-each-ref to\ntell us about everything under refs/heads/. If you have a refs/heads/foo\nand refs/tags/foo, we don't care; we are trying to print the unqualified\nbranch names. And in fact having refname:short print \"heads/foo\" in this\ncase may be actively wrong. For instance, in _git_branch(), you cannot\nuse the resulting completion of \"heads/foo\", as that command wants\nunqualified names in \"refs/heads/\", and you do not have\n\"refs/heads/heads/foo\".\n\nSo I think switching to :strip is an improvement in both correctness\n_and_ performance.\n\n> Now, if 'git for-each-ref' could understand '**' globbing, not just\n> fnmatch...\n\nI think it does already, since 4917e1e (Makefile: promote wildmatch to\nbe the default fnmatch implementation, 2013-05-30).\n\n-Peff\n"},{"id":"278054","messageId":"20160213004300.Horde.fMBUV1thpmh_xekWw-EOFAA@webmail.informatik.kit.edu","threadId":"41329","inReplyTo":"20160213002122.Horde.mxoPmZIuCikpV2PO97l11AI@webmail.informatik.kit.edu","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2016-02-12T23:43:00Z","receivedAt":"2016-02-12T23:43:00Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting SZEDER Gábor <szeder@ira.uka.de>:\n\n> Now, if 'git for-each-ref' could understand '**' globbing, not just\n> fnmatch...\n\nOh, look, though the manpage says:\n\n   <pattern>...\n       If one or more patterns are given, only refs are shown that match\n       against at least one pattern, either using fnmatch(3) or literally,\n\n'git for-each-ref' does in fact understand double asterisks:\n\n   $ git for-each-ref --format='%(refname)' '**/master'\n   refs/heads/master\n   refs/remotes/github/master\n   refs/remotes/origin/master\n   $ git for-each-ref --format='%(refname)' 'refs/heads/b/**'\n   refs/heads/b/r/a/n/c/h\n\nGreat, this combined with refname:strip=2 and 3 might open up some\nmore optimization possibilities...\n"},{"id":"278055","messageId":"20160212234655.GA23398@sigill.intra.peff.net","threadId":"41329","inReplyTo":"20160213004300.Horde.fMBUV1thpmh_xekWw-EOFAA@webmail.informatik.kit.edu","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-12T23:46:56Z","receivedAt":"2016-02-12T23:46:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 13, 2016 at 12:43:00AM +0100, SZEDER Gábor wrote:\n\n> \n> Quoting SZEDER Gábor <szeder@ira.uka.de>:\n> \n> >Now, if 'git for-each-ref' could understand '**' globbing, not just\n> >fnmatch...\n> \n> Oh, look, though the manpage says:\n> \n>   <pattern>...\n>       If one or more patterns are given, only refs are shown that match\n>       against at least one pattern, either using fnmatch(3) or literally,\n\nYeah, we might want to update that. Wildmatch is basically fnmatch()\ncompatible, but it understands \"**\" (which I _think_ is the reason we\npicked it up in the first place). I think we dropped it into place by\ndefault because \"**\" is otherwise meaningless for fnmatch.\n\nI don't think there are any other differences between the two, but Duy\nprobably knows offhand.\n\nIt looks like we mention fnmatch() in a few places in the documentation,\nand AFAIK each of these is now outdated.\n\n-Peff\n"},{"id":"278059","messageId":"CACsJy8Bg5LzXKuvastiy5WAKBR8D4iOhTcCprYqmNc3fy-HrBA@mail.gmail.com","threadId":"41329","inReplyTo":"20160212234655.GA23398@sigill.intra.peff.net","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-02-13T00:53:49Z","receivedAt":"2016-02-13T00:53:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Feb 13, 2016 at 6:46 AM, Jeff King <peff@peff.net> wrote:\n> On Sat, Feb 13, 2016 at 12:43:00AM +0100, SZEDER Gábor wrote:\n>\n>>\n>> Quoting SZEDER Gábor <szeder@ira.uka.de>:\n>>\n>> >Now, if 'git for-each-ref' could understand '**' globbing, not just\n>> >fnmatch...\n>>\n>> Oh, look, though the manpage says:\n>>\n>>   <pattern>...\n>>       If one or more patterns are given, only refs are shown that match\n>>       against at least one pattern, either using fnmatch(3) or literally,\n>\n> Yeah, we might want to update that. Wildmatch is basically fnmatch()\n> compatible, but it understands \"**\" (which I _think_ is the reason we\n> picked it up in the first place).\n\nThe second reason is consistent behavior across platforms.\n\n> I think we dropped it into place by\n> default because \"**\" is otherwise meaningless for fnmatch.\n>\n> I don't think there are any other differences between the two, but Duy\n> probably knows offhand.\n\nNope. I think that's the only difference, feature-wise, between\nfnmatch and wildmatch.\n\n> It looks like we mention fnmatch() in a few places in the documentation,\n> and AFAIK each of these is now outdated.\n-- \nDuy\n"},{"id":"278060","messageId":"20160213020712.Horde.SM-rQbc5Jx1UwdYxdvNFNJx@webmail.informatik.kit.edu","threadId":"41329","inReplyTo":"20160212234041.GA15688@sigill.intra.peff.net","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2016-02-13T01:07:12Z","receivedAt":"2016-02-13T01:07:12Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Jeff King <peff@peff.net>:\n\n> On Sat, Feb 13, 2016 at 12:21:22AM +0100, SZEDER Gábor wrote:\n>\n>> I think in this case we should opt for performance instead of correctness,\n>> and use Peff's 'refname:strip=2'.  Ambiguous refs will only hurt you, if,\n>> well, your repo actually has ambiguous refs AND you happen to want to do\n>> something with one of those refs.  I suspect that's rather uncommon, and\n>> even then you could simply rename one of those refs.  OTOH, as shown in\n>> the ticket, you don't need that many refs to make refs completion\n>> unacceptably slow on Windows, and it will bite every time you attempt to\n>> complete a ref.\n>\n> I'm not even sure that this is a correctness tradeoff at all. For\n> example, in the function __git_heads(), we are asking for-each-ref to\n> tell us about everything under refs/heads/. If you have a refs/heads/foo\n> and refs/tags/foo, we don't care; we are trying to print the unqualified\n> branch names. And in fact having refname:short print \"heads/foo\" in this\n> case may be actively wrong. For instance, in _git_branch(), you cannot\n> use the resulting completion of \"heads/foo\", as that command wants\n> unqualified names in \"refs/heads/\", and you do not have\n> \"refs/heads/heads/foo\".\n>\n> So I think switching to :strip is an improvement in both correctness\n> _and_ performance.\n\nRight.  I was more worried about __git_refs(), because it asks for\neverything under refs/heads/, refs/tags/ and refs/remotes/, and its\noutput is used in a lot more places and fed to a lot more commands than\nthe output of __git_heads() (or __git_tags(), for that matter).  But I\nthought that a branch-tag ambiguity would cause git to error out\ncomplaining, just like in the case of ref-path ambiguity.  Successfully\navoiding ambiguous refs for many years, I wasn't aware that 'git\nrev-parse' doesn't barf, but only warns and resolves the ambiguity in\nfavor of the tag.\n\n\n>> Now, if 'git for-each-ref' could understand '**' globbing, not just\n>> fnmatch...\n>\n> I think it does already, since 4917e1e (Makefile: promote wildmatch to\n> be the default fnmatch implementation, 2013-05-30).\n\nThings are looking up!\n\nA single 'master' branch and 10 remotes with 10k remote branches each,\ni.e. a total of 100001 refs, all packed.  To uniquely complete\n'master ' after 'git checkout m<TAB>' on Linux in current git.git, i.e.\nwith 'refname:short':\n\n   $ cur=m ; time __gitcomp_nl \"$(__git_refs '' 1)\"\n\n   real  0m7.641s\n   user  0m5.888s\n   sys   0m1.832s\n\nUsing 'refname:strip=2' for both 'git for-each-ref' in __git_refs():\n\n   $ cur=m ; time __gitcomp_nl \"$(__git_refs '' 1)\"\n\n   real  0m2.848s\n   user  0m2.308s\n   sys   0m0.596s\n\nQuick'n'dirty PoC using 'refname:strip', '**' globbing and a few more\ntricks to let 'git for-each-ref' do the filtering instead of the\nshell loop behind __gitcomp_nl():\n\n   $ cur=m ; time IFS=$'\\n' COMPREPLY=( $(__git_refs_PoC '' 1) )\n\n   real  0m0.247s\n   user  0m0.208s\n   sys   0m0.032s\n\nNot bad for a Friday night, huh? :)\n"},{"id":"278065","messageId":"alpine.DEB.2.20.1602131021170.2964@virtualbox","threadId":"41329","inReplyTo":"20160213020712.Horde.SM-rQbc5Jx1UwdYxdvNFNJx@webmail.informatik.kit.edu","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-02-13T09:21:52Z","receivedAt":"2016-02-13T09:21:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Gábor,\n\nOn Sat, 13 Feb 2016, SZEDER Gábor wrote:\n\n>  $ cur=m ; time __gitcomp_nl \"$(__git_refs '' 1)\"\n> \n>  real  0m7.641s\n>  user  0m5.888s\n>  sys   0m1.832s\n> \n> Using 'refname:strip=2' for both 'git for-each-ref' in __git_refs():\n> \n>  $ cur=m ; time __gitcomp_nl \"$(__git_refs '' 1)\"\n> \n>  real  0m2.848s\n>  user  0m2.308s\n>  sys   0m0.596s\n> \n> Quick'n'dirty PoC using 'refname:strip', '**' globbing and a few more\n> tricks to let 'git for-each-ref' do the filtering instead of the\n> shell loop behind __gitcomp_nl():\n> \n>  $ cur=m ; time IFS=$'\\n' COMPREPLY=( $(__git_refs_PoC '' 1) )\n> \n>  real  0m0.247s\n>  user  0m0.208s\n>  sys   0m0.032s\n> \n> Not bad for a Friday night, huh? :)\n\nNope, not bad at all. May I have that patch, please? ;-)\n\nCiao,\nDscho"},{"id":"278067","messageId":"20160213145333.Horde.ZTzk8ajnzz2uB2UcNeCdPtB@webmail.informatik.kit.edu","threadId":"41329","inReplyTo":"alpine.DEB.2.20.1602131021170.2964@virtualbox","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2016-02-13T13:53:33Z","receivedAt":"2016-02-13T13:53:33Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Johannes Schindelin <Johannes.Schindelin@gmx.de>:\n\n> Hi Gábor,\n>\n> On Sat, 13 Feb 2016, SZEDER Gábor wrote:\n>\n>>  $ cur=m ; time __gitcomp_nl \"$(__git_refs '' 1)\"\n>>\n>>  real  0m7.641s\n>>  user  0m5.888s\n>>  sys   0m1.832s\n>>\n>> Using 'refname:strip=2' for both 'git for-each-ref' in __git_refs():\n>>\n>>  $ cur=m ; time __gitcomp_nl \"$(__git_refs '' 1)\"\n>>\n>>  real  0m2.848s\n>>  user  0m2.308s\n>>  sys   0m0.596s\n\nI timed this one using a version that already included one from those\n\"few more tricks\", so the change from ':short' to ':strip=2' alone\ndoesn't bring quite as much:\n\n   $ cur=m ; time __gitcomp_nl \"$(__git_refs '' 1)\"\n\n   real  0m3.645s\n   user  0m3.140s\n   sys   0m0.588s\n\n\n>> Quick'n'dirty PoC using 'refname:strip', '**' globbing and a few more\n>> tricks to let 'git for-each-ref' do the filtering instead of the\n>> shell loop behind __gitcomp_nl():\n>>\n>>  $ cur=m ; time IFS=$'\\n' COMPREPLY=( $(__git_refs_PoC '' 1) )\n>>\n>>  real  0m0.247s\n>>  user  0m0.208s\n>>  sys   0m0.032s\n\nAnd this one now looks like:\n\n   $ cur=m ; time __gitcomp_direct \"$(__git_refs_PoC '' 1)\"\n\nThe timing results are the same.\n\n\n> May I have that patch, please? ;-)\n\nIt's early days, and when I say proof of concept I mean it :)\nFor now it only works for refs from the local repository, and only\nwhen the ref to be completed is on its own on the command line (i.e.\nnot for 'git log master..<TAB>' or 'commit --fixup=<TAB>'), and the\ntrailing space is hardcoded, and ...  though, arguably, that already\ncovers the majority of the cases.  I only switched 'git checkout' to\nuse this optimized version, because that was the worst offender.\n\nSo I won't send patches to the list just now, but you or anyone\ninterested can take a peek at:\n\n   https://github.com/szeder/git.git completion-PoC-refs-speedup\n\nMaybe even run some numbers on Windows?\n\n\nGábor\n"},{"id":"278073","messageId":"20160213165722.GA30144@sigill.intra.peff.net","threadId":"41329","inReplyTo":"20160213020712.Horde.SM-rQbc5Jx1UwdYxdvNFNJx@webmail.informatik.kit.edu","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-13T16:57:23Z","receivedAt":"2016-02-13T16:57:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 13, 2016 at 02:07:12AM +0100, SZEDER Gábor wrote:\n\n> >So I think switching to :strip is an improvement in both correctness\n> >_and_ performance.\n> \n> Right.  I was more worried about __git_refs(), because it asks for\n> everything under refs/heads/, refs/tags/ and refs/remotes/, and its\n> output is used in a lot more places and fed to a lot more commands than\n> the output of __git_heads() (or __git_tags(), for that matter).  But I\n> thought that a branch-tag ambiguity would cause git to error out\n> complaining, just like in the case of ref-path ambiguity.  Successfully\n> avoiding ambiguous refs for many years, I wasn't aware that 'git\n> rev-parse' doesn't barf, but only warns and resolves the ambiguity in\n> favor of the tag.\n\nYeah, switching to :strip would arguably be a regression when completing\nall refs. Right now, you'd get \"heads/foo\" and \"tags/foo\" as part of\nyour completion (but _not_ just \"foo\"), and either works as a\nnon-ambiguous ref.\n\nWith :strip, you'd just get \"foo\" twice, and if you use the result of\nthe completion, it will always point to the tag.\n\nSo it is arguably worse. I still think it is worth trading off for\nperformance, but it is worth acknowledging in the commit message there\nthat it is a tradeoff.\n\n> >I think it does already, since 4917e1e (Makefile: promote wildmatch to\n> >be the default fnmatch implementation, 2013-05-30).\n> \n> Things are looking up!\n> [...vast improvement in times...]\n\nVery cool. I look forward to seeing the final patch. :)\n\nI have noticed in my pathological 10-million-ref bare repositories\n(don't ask) that the __git_ps1() prompt is quite slow, too. And I\nwondered if it could be related.\n\nBut I don't think it is. It's just literally that painful to look at the\npacked-refs at all, and \"git rev-parse HEAD\" has to look at them to\nresolve.\n\n-Peff\n"},{"id":"278075","messageId":"alpine.DEB.2.20.1602131811570.2964@virtualbox","threadId":"41329","inReplyTo":"20160213145333.Horde.ZTzk8ajnzz2uB2UcNeCdPtB@webmail.informatik.kit.edu","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-02-13T17:14:51Z","receivedAt":"2016-02-13T17:14:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Gábor,\n\nOn Sat, 13 Feb 2016, SZEDER Gábor wrote:\n\n> Maybe even run some numbers on Windows?\n\nHere you are:\n\n-- snip --\n# Junio's 494398473, i.e. `master\n$ cur=m ; time __gitcomp_nl \"$(__git_refs '' 1)\"\n\nreal    0m8.260s\nuser    0m0.265s\nsys     0m0.216s\n\n# Gábor's 6fc6f416, i.e. with strip=2\n$ cur=m ; time __gitcomp_nl \"$(__git_refs '' 1)\"\n\nreal    0m2.072s\nuser    0m0.295s\nsys     0m0.203s\n\n# Gábor's 47b146d7, i.e. `completion-PoC-refs-speedup`\n$ cur=m ; time __gitcomp_direct \"$(__git_refs_PoC '' 1)\"\n\nreal    0m1.574s\nuser    0m0.030s\nsys     0m0.015s\n-- snap --\n\nThis is with a fairly normal, real-world repository:\n\n-- snip --\n$ git for-each-ref | wc -l\n10303\n-- snap --\n\nCiao,\nDscho"},{"id":"279092","messageId":"xmqqfuwjngwy.fsf@gitster.mtv.corp.google.com","threadId":"41329","inReplyTo":"xmqqk2m9ts91.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-23T23:30:37Z","receivedAt":"2016-02-23T23:30:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> On Fri, Feb 12, 2016 at 10:40:48PM +0100, SZEDER Gábor wrote:\n>>\n>>>  * It would swallow even those errors that we are interested in,\n>>>    e.g. (note the missing quotes around $foo):\n>>> [...]\n>>>  * I often find myself tracing/debugging the completion script\n>>>    through stderr by scattering\n>>> \n>>>       echo >&2 \"foo: '$foo'\"\n>>\n>> One alternative to deal with these would be to add a flag to\n>> conditionally turn off stderr, and then leave it on during normal\n>> operation and disable it (letting everything through, including whatever\n>> random cruft git commands produce) for debugging.\n>>\n>> But...\n>>\n>>>  * I have a WIP patch series that deals with errors from git\n>>>    commands.\n>>\n>> I'm happy to wait and see what this patch looks like (and generally\n>> happy to defer to you on maintenance issues for completion, as you are\n>> much more likely than me to be the one fixing things later on :) ).\n>>\n>> -Peff\n>\n> Likewise on both counts.\n\nSo, have we decided to wait, or we'd rather apply the band-aid in\nthe meantime?  I can go either way, just double checking as I\nnoticed this thread while updating my leftover bits list.\n\nThanks.\n"},{"id":"279132","messageId":"CAHGBnuPcSFVknueuO5zTo9i956dZKyo+mXga9YCN8XByxZg=8A@mail.gmail.com","threadId":"41329","inReplyTo":"xmqqfuwjngwy.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2016-02-24T07:48:52Z","receivedAt":"2016-02-24T07:48:52Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Wed, Feb 24, 2016 at 12:30 AM, Junio C Hamano <gitster@pobox.com> wrote:\n\n> So, have we decided to wait, or we'd rather apply the band-aid in\n> the meantime?  I can go either way, just double checking as I\n> noticed this thread while updating my leftover bits list.\n\nThanks for the follow-up, I was about to ask for a status update on\nthis. As my patch it ready now, and we don't know how long we'd have\nto wait for the other solution, I'd vote for applying my patch.\n\n-- \nSebastian Schuberth\n"}]}