{"thread":{"id":"38540","subject":"[PATCH] contrib/completion: suppress stderror in bash completion of git remotes","startedAt":"2015-02-09T20:58:28Z","lastAt":"2015-03-04T14:10:29Z","messageCount":11,"participants":["Matt Korostoff","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"255850","messageId":"1423515508-40109-1-git-send-email-MKorostoff@gmail.com","threadId":"38540","inReplyTo":null,"subject":"[PATCH] contrib/completion: suppress stderror in bash completion of git remotes","fromName":"Matt Korostoff","fromEmail":"mkorostoff@gmail.com","sentAt":"2015-02-09T20:58:28Z","receivedAt":"2015-02-09T20:58:28Z","isPatch":true,"sender":{"key":"mkorostoff@gmail.com","avatar":"https://gravatar.com/avatar/440d0e9d69a2e351c8c28e202e1d7223e8a95666cedf1a7b8654aef62774864e?d=mp&s=160"},"body":"In some system configurations there is a bug with the\n__git_remotes function.  Specifically, there is a problem\nwith line 415, `test -d \"$d/remotes\" && ls -1 \"$d/remotes\"`.\nWhile `test -d` is meant to prevent listing the remotes\ndirectory if it does not exist, in some system, `ls` will\nrun regardless.\n\nThis results in an error in which typing `git push or` + `tab`\nprints out `ls: .git/remotes: No such file or directory`.\nThis can be fixed by simply directing stderror of this line\nto /dev/null.\n---\n contrib/completion/git-completion.bash |    2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 2fece98..72251cc 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -412,7 +412,7 @@ __git_refs_remotes ()\n __git_remotes ()\n {\n \tlocal i IFS=$'\\n' d=\"$(__gitdir)\"\n-\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n+\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\" 2>/dev/null\n \tfor i in $(git --git-dir=\"$d\" config --get-regexp 'remote\\..*\\.url' 2>/dev/null); do\n \t\ti=\"${i#remote.}\"\n \t\techo \"${i/.url*/}\"\n-- \n1.7.10.2 (Apple Git-33)\n"},{"id":"255851","messageId":"CAMZO7dL-ENGmjt4J-rb5y2yVOFGVQKXdh3UB5krnJyfU5M=6xw@mail.gmail.com","threadId":"38540","inReplyTo":"1423515508-40109-1-git-send-email-MKorostoff@gmail.com","subject":"Re: [PATCH] contrib/completion: suppress stderror in bash completion of git remotes","fromName":"Matt Korostoff","fromEmail":"mkorostoff@gmail.com","sentAt":"2015-02-09T21:00:12Z","receivedAt":"2015-02-09T21:00:12Z","isPatch":true,"sender":{"key":"mkorostoff@gmail.com","avatar":"https://gravatar.com/avatar/440d0e9d69a2e351c8c28e202e1d7223e8a95666cedf1a7b8654aef62774864e?d=mp&s=160"},"body":"Here are some screen shots demonstrating the issue I'm describing here:\n\nbefore this patch:\nhttps://cloud.githubusercontent.com/assets/1197335/6108333/1f3b10fa-b040-11e4-9164-3c7769dae110.gif\n\nafter this patch:\nhttps://cloud.githubusercontent.com/assets/1197335/6108340/3878cad0-b040-11e4-9994-dcd5c4d62bba.gif\n\nOn Mon, Feb 9, 2015 at 3:58 PM, Matt Korostoff <mkorostoff@gmail.com> wrote:\n> In some system configurations there is a bug with the\n> __git_remotes function.  Specifically, there is a problem\n> with line 415, `test -d \"$d/remotes\" && ls -1 \"$d/remotes\"`.\n> While `test -d` is meant to prevent listing the remotes\n> directory if it does not exist, in some system, `ls` will\n> run regardless.\n>\n> This results in an error in which typing `git push or` + `tab`\n> prints out `ls: .git/remotes: No such file or directory`.\n> This can be fixed by simply directing stderror of this line\n> to /dev/null.\n> ---\n>  contrib/completion/git-completion.bash |    2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 2fece98..72251cc 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -412,7 +412,7 @@ __git_refs_remotes ()\n>  __git_remotes ()\n>  {\n>         local i IFS=$'\\n' d=\"$(__gitdir)\"\n> -       test -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n> +       test -d \"$d/remotes\" && ls -1 \"$d/remotes\" 2>/dev/null\n>         for i in $(git --git-dir=\"$d\" config --get-regexp 'remote\\..*\\.url' 2>/dev/null); do\n>                 i=\"${i#remote.}\"\n>                 echo \"${i/.url*/}\"\n> --\n> 1.7.10.2 (Apple Git-33)\n>\n"},{"id":"255852","messageId":"xmqqy4o6aj1w.fsf@gitster.dls.corp.google.com","threadId":"38540","inReplyTo":"1423515508-40109-1-git-send-email-MKorostoff@gmail.com","subject":"Re: [PATCH] contrib/completion: suppress stderror in bash completion of git remotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-09T21:09:31Z","receivedAt":"2015-02-09T21:09:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matt Korostoff <mkorostoff@gmail.com> writes:\n\n> In some system configurations there is a bug with the\n> __git_remotes function.  Specifically, there is a problem\n> with line 415, `test -d \"$d/remotes\" && ls -1 \"$d/remotes\"`.\n> While `test -d` is meant to prevent listing the remotes\n> directory if it does not exist, in some system, `ls` will\n> run regardless.\n\nWhat's \"some system\"?\n\nIs this a platform's bug (e.g. \"test -d\" does not work correctly)?\n\nIs this an configuration error of user's Git repository?\n\nIs this something else?\n\nI _think_ you would see the problem if $d/remotes is a directory\nwhose contents cannot be listed (e.g. \"chmod a= $d/remotes\"), and\nthat would not be a platform's bug (i.e. \"test -d\" would happily say\n\"Yes there is a directory\", and \"ls\" would fail with \"Permission\ndenied\").  But I wonder if we rather want the user to notice that\nmisconfiguration so that the user can correct it, instead of hiding\nthe error message from \"ls\".\n\n> This results in an error in which typing `git push or` + `tab`\n> prints out `ls: .git/remotes: No such file or directory`.\n> This can be fixed by simply directing stderror of this line\n> to /dev/null.\n> ---\n>  contrib/completion/git-completion.bash |    2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 2fece98..72251cc 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -412,7 +412,7 @@ __git_refs_remotes ()\n>  __git_remotes ()\n>  {\n>  \tlocal i IFS=$'\\n' d=\"$(__gitdir)\"\n> -\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n> +\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\" 2>/dev/null\n>  \tfor i in $(git --git-dir=\"$d\" config --get-regexp 'remote\\..*\\.url' 2>/dev/null); do\n>  \t\ti=\"${i#remote.}\"\n>  \t\techo \"${i/.url*/}\"\n"},{"id":"255872","messageId":"CAMZO7d+qL-rzvi0GrAuzAg=02MxByMt0Zc=fMnvK175YQyB8YA@mail.gmail.com","threadId":"38540","inReplyTo":"xmqqy4o6aj1w.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] contrib/completion: suppress stderror in bash completion of git remotes","fromName":"Matt Korostoff","fromEmail":"mkorostoff@gmail.com","sentAt":"2015-02-10T00:08:09Z","receivedAt":"2015-02-10T00:08:09Z","isPatch":true,"sender":{"key":"mkorostoff@gmail.com","avatar":"https://gravatar.com/avatar/440d0e9d69a2e351c8c28e202e1d7223e8a95666cedf1a7b8654aef62774864e?d=mp&s=160"},"body":"Thank you for your detailed reply Junio.  I'll try to address your\nconcerns individually, but let me also offer a general thought that\nthis is probably a good use case to handle even if the root cause is\nsolvable outside of git.  That is to say, I would think we'd still\nwant git autocompletion working for users running on imperfect\nplatforms.\n\n> What's \"some system\"?\n\nMy apologies, that was a typo. I meant to write \"some systems.\"  I'm\nnot sure what the root cause is, but I can tell you I'm running a\nrather vanilla development environment (git 1.7.10, bash 3.2, osx\n10.8).  I wish I could supply a list of the version combinations that\nresult in such an event, but I'm not sure how I would do acquire a\nlist like that.\n\n> Is this a platform's bug (e.g. \"test -d\" does not work correctly)?\n\nI don't believe so—here's a simple test-of-the-test I threw together\nhttps://gist.github.com/MKorostoff/f203e414847d43b21de4 which does not\nthrow this error.\n\n> Is this an configuration error of user's Git repository?\n\nI think I have a pretty generic git configuration (here it is, though\nnote I've had to redact some identifying information\nhttps://gist.github.com/MKorostoff/f8358f72b968249a3925).  Still, I'd\nthink we would want to handle such a misconfiguration explicitly,\nrather than throw a seemingly unrelated error during autocompletion\n\n> Is this something else?\n\nIt would be very helpful if you could supply a few more details on the\ntype of something you're looking for\n\n> I _think_ you would see the problem if $d/remotes is a directory\n> whose contents cannot be listed\n\nI can confirm, that is indeed the behavior.  Animated gif example here\nhttp://i.imgur.com/qcPxAub.gif\n\n> But I wonder if we rather want the user to notice that\n> misconfiguration so that the user can correct it\n\nWhile I wholeheartedly agree that such user feedback would be\nvaluable, I'm just not sure that it makes sense for this feedback to\ninterrupt the user's typing mid-word.\n\nSorry if anyone receives this twice, my first attempt to send was\nrejected for including HTML.\n\nOn Mon, Feb 9, 2015 at 4:09 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Matt Korostoff <mkorostoff@gmail.com> writes:\n>\n>> In some system configurations there is a bug with the\n>> __git_remotes function.  Specifically, there is a problem\n>> with line 415, `test -d \"$d/remotes\" && ls -1 \"$d/remotes\"`.\n>> While `test -d` is meant to prevent listing the remotes\n>> directory if it does not exist, in some system, `ls` will\n>> run regardless.\n>\n> What's \"some system\"?\n>\n> Is this a platform's bug (e.g. \"test -d\" does not work correctly)?\n>\n> Is this an configuration error of user's Git repository?\n>\n> Is this something else?\n>\n> I _think_ you would see the problem if $d/remotes is a directory\n> whose contents cannot be listed (e.g. \"chmod a= $d/remotes\"), and\n> that would not be a platform's bug (i.e. \"test -d\" would happily say\n> \"Yes there is a directory\", and \"ls\" would fail with \"Permission\n> denied\").  But I wonder if we rather want the user to notice that\n> misconfiguration so that the user can correct it, instead of hiding\n> the error message from \"ls\".\n>\n>> This results in an error in which typing `git push or` + `tab`\n>> prints out `ls: .git/remotes: No such file or directory`.\n>> This can be fixed by simply directing stderror of this line\n>> to /dev/null.\n>> ---\n>>  contrib/completion/git-completion.bash |    2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>> index 2fece98..72251cc 100644\n>> --- a/contrib/completion/git-completion.bash\n>> +++ b/contrib/completion/git-completion.bash\n>> @@ -412,7 +412,7 @@ __git_refs_remotes ()\n>>  __git_remotes ()\n>>  {\n>>       local i IFS=$'\\n' d=\"$(__gitdir)\"\n>> -     test -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n>> +     test -d \"$d/remotes\" && ls -1 \"$d/remotes\" 2>/dev/null\n>>       for i in $(git --git-dir=\"$d\" config --get-regexp 'remote\\..*\\.url' 2>/dev/null); do\n>>               i=\"${i#remote.}\"\n>>               echo \"${i/.url*/}\"\n"},{"id":"255875","messageId":"20150210031008.Horde.1WYJ_81O2E96Rgv2xrVmqw1@webmail.informatik.kit.edu","threadId":"38540","inReplyTo":"xmqqy4o6aj1w.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] contrib/completion: suppress stderror in bash","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-02-10T02:10:08Z","receivedAt":"2015-02-10T02:10:08Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nHi,\n\n\nQuoting Junio C Hamano <gitster@pobox.com>:\n\n> Matt Korostoff <mkorostoff@gmail.com> writes:\n\n>> diff --git a/contrib/completion/git-completion.bash\n>> b/contrib/completion/git-completion.bash\n>> index 2fece98..72251cc 100644\n>> --- a/contrib/completion/git-completion.bash\n>> +++ b/contrib/completion/git-completion.bash\n>> @@ -412,7 +412,7 @@ __git_refs_remotes ()\n>>  __git_remotes ()\n>>  {\n>>  \tlocal i IFS=$'\\n' d=\"$(__gitdir)\"\n>> -\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n>> +\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\" 2>/dev/null\n>>  \tfor i in $(git --git-dir=\"$d\" config --get-regexp\n>> 'remote\\..*\\.url' 2>/dev/null); do\n>>  \t\ti=\"${i#remote.}\"\n>>  \t\techo \"${i/.url*/}\"\n\nDo I smell some bitrotting here?\n\nThis function just lists all the defined remotes, first by listing the  \ndirectories under refs/remotes to get the \"legacy\" remotes and then  \nloops over 'git config's output to get the \"modern\" ones.  This  \npredates the arrival of the 'git remote' command in January 2007, so  \nit was really a long time ago.\n\nWe should just run 'git remote' instead, shouldn't we?\n\n\nCheers,\nGábor\n"},{"id":"255885","messageId":"1423581907-88600-1-git-send-email-MKorostoff@gmail.com","threadId":"38540","inReplyTo":"20150210031008.Horde.1WYJ_81O2E96Rgv2xrVmqw1@webmail.informatik.kit.edu","subject":"[PATCH] contrib/completion: deprecate __git_remotes in bash completion","fromName":"Matt Korostoff","fromEmail":"mkorostoff@gmail.com","sentAt":"2015-02-10T15:25:07Z","receivedAt":"2015-02-10T15:25:07Z","isPatch":true,"sender":{"key":"mkorostoff@gmail.com","avatar":"https://gravatar.com/avatar/440d0e9d69a2e351c8c28e202e1d7223e8a95666cedf1a7b8654aef62774864e?d=mp&s=160"},"body":"Bash auto completion supplies a function __git_remotes which\nlists the git remotes of a repository by reading the\n.git/remotes directory.  As of git 1.7.6 this is handled\nnatively by the `git remote` command.  This function is\nnow deprecated.\n\nSigned-off-by: Matt Korostoff <MKorostoff@gmail.com>\n---\n\n*****\n\nGreat point Gabor! I would hesitate a little about removing the function \nentirely, because users may have external scripts that rely on this function,\nbut certainly for our internal purposes the __git_remotes shell function can be\nreplaced with the native.  Here's a short screen cast of the included patch \nworking on my local http://i.imgur.com/6ZSMXCO.gif\n\n*****\n\n contrib/completion/git-completion.bash |   30 +++++++++++++++---------------\n 1 file changed, 15 insertions(+), 15 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 2fece98..8b41871 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -409,14 +409,14 @@ __git_refs_remotes ()\n \tdone\n }\n \n+# Deprecated wrapper function around git remote.  Supplied for legacy purposes,\n+# to avoid breaking any external scripts which rely on this function.\n+# Originally this function was used to programmatically generate a list of git\n+# remotes by reading the .git/remotes directory, but as of git 1.7.6 that is\n+# natively handled by the git remote command\n __git_remotes ()\n {\n-\tlocal i IFS=$'\\n' d=\"$(__gitdir)\"\n-\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n-\tfor i in $(git --git-dir=\"$d\" config --get-regexp 'remote\\..*\\.url' 2>/dev/null); do\n-\t\ti=\"${i#remote.}\"\n-\t\techo \"${i/.url*/}\"\n-\tdone\n+\tgit remote\n }\n \n __git_list_merge_strategies ()\n@@ -558,7 +558,7 @@ __git_complete_remote_or_refspec ()\n \t\t((c++))\n \tdone\n \tif [ -z \"$remote\" ]; then\n-\t\t__gitcomp_nl \"$(__git_remotes)\"\n+\t\t__gitcomp_nl \"$(git remote)\"\n \t\treturn\n \tfi\n \tif [ $no_complete_refspec = 1 ]; then\n@@ -927,7 +927,7 @@ _git_archive ()\n \t\treturn\n \t\t;;\n \t--remote=*)\n-\t\t__gitcomp_nl \"$(__git_remotes)\" \"\" \"${cur##--remote=}\"\n+\t\t__gitcomp_nl \"$(git remote)\" \"\" \"${cur##--remote=}\"\n \t\treturn\n \t\t;;\n \t--*)\n@@ -1397,7 +1397,7 @@ _git_ls_files ()\n \n _git_ls_remote ()\n {\n-\t__gitcomp_nl \"$(__git_remotes)\"\n+\t__gitcomp_nl \"$(git remote)\"\n }\n \n _git_ls_tree ()\n@@ -1639,7 +1639,7 @@ _git_push ()\n {\n \tcase \"$prev\" in\n \t--repo)\n-\t\t__gitcomp_nl \"$(__git_remotes)\"\n+\t\t__gitcomp_nl \"$(git remote)\"\n \t\treturn\n \t\t;;\n \t--recurse-submodules)\n@@ -1649,7 +1649,7 @@ _git_push ()\n \tesac\n \tcase \"$cur\" in\n \t--repo=*)\n-\t\t__gitcomp_nl \"$(__git_remotes)\" \"\" \"${cur##--repo=}\"\n+\t\t__gitcomp_nl \"$(git remote)\" \"\" \"${cur##--repo=}\"\n \t\treturn\n \t\t;;\n \t--recurse-submodules=*)\n@@ -1797,7 +1797,7 @@ _git_config ()\n {\n \tcase \"$prev\" in\n \tbranch.*.remote|branch.*.pushremote)\n-\t\t__gitcomp_nl \"$(__git_remotes)\"\n+\t\t__gitcomp_nl \"$(git remote)\"\n \t\treturn\n \t\t;;\n \tbranch.*.merge)\n@@ -1809,7 +1809,7 @@ _git_config ()\n \t\treturn\n \t\t;;\n \tremote.pushdefault)\n-\t\t__gitcomp_nl \"$(__git_remotes)\"\n+\t\t__gitcomp_nl \"$(git remote)\"\n \t\treturn\n \t\t;;\n \tremote.*.fetch)\n@@ -1944,7 +1944,7 @@ _git_config ()\n \t\t;;\n \tremote.*)\n \t\tlocal pfx=\"${cur%.*}.\" cur_=\"${cur#*.}\"\n-\t\t__gitcomp_nl \"$(__git_remotes)\" \"$pfx\" \"$cur_\" \".\"\n+\t\t__gitcomp_nl \"$(git remote)\" \"$pfx\" \"$cur_\" \".\"\n \t\t__gitcomp_nl_append \"pushdefault\" \"$pfx\" \"$cur_\"\n \t\treturn\n \t\t;;\n@@ -2250,7 +2250,7 @@ _git_remote ()\n \n \tcase \"$subcommand\" in\n \trename|remove|set-url|show|prune)\n-\t\t__gitcomp_nl \"$(__git_remotes)\"\n+\t\t__gitcomp_nl \"$(git remote)\"\n \t\t;;\n \tset-head|set-branches)\n \t\t__git_complete_remote_or_refspec\n-- \n1.7.10.2 (Apple Git-33)\n"},{"id":"255890","messageId":"xmqqegpxwrdc.fsf@gitster.dls.corp.google.com","threadId":"38540","inReplyTo":"20150210031008.Horde.1WYJ_81O2E96Rgv2xrVmqw1@webmail.informatik.kit.edu","subject":"Re: [PATCH] contrib/completion: suppress stderror in bash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-10T18:31:11Z","receivedAt":"2015-02-10T18:31:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder@ira.uka.de> writes:\n\n>>> @@ -412,7 +412,7 @@ __git_refs_remotes ()\n>>>  __git_remotes ()\n>>>  {\n>>>  \tlocal i IFS=$'\\n' d=\"$(__gitdir)\"\n>>> -\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n>>> +\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\" 2>/dev/null\n>>>  \tfor i in $(git --git-dir=\"$d\" config --get-regexp\n>>> 'remote\\..*\\.url' 2>/dev/null); do\n>>>  \t\ti=\"${i#remote.}\"\n>>>  \t\techo \"${i/.url*/}\"\n>\n> Do I smell some bitrotting here?\n>\n> This function just lists all the defined remotes, first by listing the  \n> directories under refs/remotes to get the \"legacy\" remotes and then  \n> loops over 'git config's output to get the \"modern\" ones.  This  \n> predates the arrival of the 'git remote' command in January 2007, so  \n> it was really a long time ago.\n>\n> We should just run 'git remote' instead, shouldn't we?\n\nPerhaps.  Is it sufficient to just make __git_remotes() a thin\nwrapper around, i.e.\n\n\t__git_remotes ()\n\t{\n\t\tgit remotes\n\t}\n\nor do we need to munge its output further (I didn't look)?\n"},{"id":"255896","messageId":"20150210201648.Horde.QMVdqmJIJnvLkf0U46NdYA1@webmail.informatik.kit.edu","threadId":"38540","inReplyTo":"xmqqegpxwrdc.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] contrib/completion: suppress stderror in bash","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-02-10T19:16:48Z","receivedAt":"2015-02-10T19:16:48Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nHi,\n\nQuoting Junio C Hamano <gitster@pobox.com>:\n\n> SZEDER Gábor <szeder@ira.uka.de> writes:\n>\n>>>> @@ -412,7 +412,7 @@ __git_refs_remotes ()\n>>>> __git_remotes ()\n>>>> {\n>>>> \tlocal i IFS=$'\\n' d=\"$(__gitdir)\"\n>>>> -\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n>>>> +\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\" 2>/dev/null\n>>>> \tfor i in $(git --git-dir=\"$d\" config --get-regexp\n>>>> 'remote\\..*\\.url' 2>/dev/null); do\n>>>> \t\ti=\"${i#remote.}\"\n>>>> \t\techo \"${i/.url*/}\"\n>>\n>> Do I smell some bitrotting here?\n>>\n>> This function just lists all the defined remotes, first by listing the\n>> directories under refs/remotes to get the \"legacy\" remotes and then\n>> loops over 'git config's output to get the \"modern\" ones.  This\n>> predates the arrival of the 'git remote' command in January 2007, so\n>> it was really a long time ago.\n>>\n>> We should just run 'git remote' instead, shouldn't we?\n>\n> Perhaps.  Is it sufficient to just make __git_remotes() a thin\n> wrapper around, i.e.\n>\n> \t__git_remotes ()\n> \t{\n> \t\tgit remotes\n> \t}\n>\n> or do we need to munge its output further (I didn't look)?\n\nWell, just like in other cases where we run git from the completion\nscript, we need a '--git-dir=\"$(__gitdir)\"' as well, because the user can\nspecify the path to a different repo via $GIT_DIR or on the command\nline.\nOther than that it seems we are OK.  Docs say \"With no arguments,\nshows a list of existing remotes.\" and as far as I can tell, on\nMSysGit, it does so without any funny formatting.\n\nBut beware, I spent most of last year cycling to Mongolia and my git-fu\nbecame a somewhat rusty... and I'm not quite up to speed yet.\n\nBest,\nGábor\n"},{"id":"256999","messageId":"20150304150447.Horde.t4rJ5Q_QdRg1uznQKWetMg1@webmail.informatik.kit.edu","threadId":"38540","inReplyTo":"20150210201648.Horde.QMVdqmJIJnvLkf0U46NdYA1@webmail.informatik.kit.edu","subject":"Re: [PATCH] contrib/completion: suppress stderror in bash","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-03-04T14:04:47Z","receivedAt":"2015-03-04T14:04:47Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n\nQuoting SZEDER Gábor <szeder@ira.uka.de>:\n\n> Hi,\n>\n> Quoting Junio C Hamano <gitster@pobox.com>:\n>\n>> SZEDER Gábor <szeder@ira.uka.de> writes:\n>>\n>>>>> @@ -412,7 +412,7 @@ __git_refs_remotes ()\n>>>>> __git_remotes ()\n>>>>> {\n>>>>> \tlocal i IFS=$'\\n' d=\"$(__gitdir)\"\n>>>>> -\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n>>>>> +\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\" 2>/dev/null\n>>>>> \tfor i in $(git --git-dir=\"$d\" config --get-regexp\n>>>>> 'remote\\..*\\.url' 2>/dev/null); do\n>>>>> \t\ti=\"${i#remote.}\"\n>>>>> \t\techo \"${i/.url*/}\"\n>>>\n>>> Do I smell some bitrotting here?\n>>>\n>>> This function just lists all the defined remotes, first by listing the\n>>> directories under refs/remotes to get the \"legacy\" remotes and then\n>>> loops over 'git config's output to get the \"modern\" ones.  This\n>>> predates the arrival of the 'git remote' command in January 2007, so\n>>> it was really a long time ago.\n>>>\n>>> We should just run 'git remote' instead, shouldn't we?\n>>\n>> Perhaps.  Is it sufficient to just make __git_remotes() a thin\n>> wrapper around, i.e.\n>>\n>> \t__git_remotes ()\n>> \t{\n>> \t\tgit remotes\n>> \t}\n>>\n>> or do we need to munge its output further (I didn't look)?\n>\n> Well, just like in other cases where we run git from the completion\n> script, we need a '--git-dir=\"$(__gitdir)\"' as well, because the user can\n> specify the path to a different repo via $GIT_DIR or on the command\n> line.\n> Other than that it seems we are OK.  Docs say \"With no arguments,\n> shows a list of existing remotes.\" and as far as I can tell, on\n> MSysGit, it does so without any funny formatting.\n\nOh, look what forgotten treasure did I stumble upon in the vaults:\n\n    \nhttps://github.com/szeder/git/commit/e4e3760c15b485b9ff4768e13050f4b19b5968b8\n\nA two and a half year old commit in my old git repo doing the same...   \ncompletely forgotten :)\n\nUnfortunately, however, it's not quite that simple, because 'git  \nremote' doesn't list remotes under '$GIT_DIR/remotes'.  Or at least I  \nwould have expected the following test to work, but it does not:\n\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 17c6330..6a4c139 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -734,6 +734,15 @@ Pull: refs/heads/master:refs/heads/origin\n  Pull: refs/heads/next:refs/heads/origin2\n  EOF\n\n+test_expect_success 'list remote in $GIT_DIR/remotes' '\n+\tmkdir .git/remotes &&\n+\ttest_when_finished \"rm -rf .git/remotes\" &&\n+\tcat remotes_origin >.git/remotes/remote_from_file &&\n+\tgit remote >actual &&\n+\techo remote_from_file >expect &&\n+\ttest_cmp expect actual\n+'\n+\n  test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' '\n  \tgit clone one five &&\n  \torigin_url=$(pwd)/one &&\n\nbecause listing remotes is implemented by for_each_remote(), which  \nonly reads remotes from the config file.\n\nNow, considering how old 'git remote' is, there were plenty of time  \nfor someone to miss this functionality and complain about it, but  \nsince it's still not implemented is probably a good sign that noone  \nhas actually missed it.  And I don't think it's worth implementing it  \nnow just to shave off two more lines from the completion script.\n\nAnyway, 'git remote' could still replace that 'git config' query.  I  \nhave the patches ready and it seems I got send-email working, so  \nthey'll follow in a minute or two.\n\nBest,\nGábor\n"},{"id":"257001","messageId":"1425478229-11252-1-git-send-email-szeder@ira.uka.de","threadId":"38540","inReplyTo":"20150304150447.Horde.t4rJ5Q_QdRg1uznQKWetMg1@webmail.informatik.kit.edu","subject":"[PATCH 1/2] completion: add a test for __git_remotes() helper function","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-03-04T14:10:28Z","receivedAt":"2015-03-04T14:10:28Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The test checks that both remotes under '$GIT_DIR/remotes' and remotes\nin the config file are listed.\n\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\n t/t9902-completion.sh | 19 +++++++++++++++++++\n 1 file changed, 19 insertions(+)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex f10a752..7a883d1 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -351,6 +351,25 @@ test_expect_success '__gitcomp_nl - doesnt fail because of invalid variable name\n \t__gitcomp_nl \"$invalid_variable_name\"\n '\n \n+test_expect_success '__git_remotes - list remotes from $GIT_DIR/remotes and from config file' '\n+\tcat >expect <<-EOF &&\n+\tremote_from_file_1\n+\tremote_from_file_2\n+\tremote_in_config_1\n+\tremote_in_config_2\n+\tEOF\n+\ttest_when_finished \"rm -rf .git/remotes\" &&\n+\tmkdir -p .git/remotes &&\n+\t>.git/remotes/remote_from_file_1 &&\n+\t>.git/remotes/remote_from_file_2 &&\n+\ttest_when_finished \"git remote remove remote_in_config_1\" &&\n+\tgit remote add remote_in_config_1 git://remote_1 &&\n+\ttest_when_finished \"git remote remove remote_in_config_2\" &&\n+\tgit remote add remote_in_config_2 git://remote_2 &&\n+\t__git_remotes >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'basic' '\n \trun_completion \"git \" &&\n \t# built-in\n-- \n2.1.2.1369.g8751039\n"},{"id":"257000","messageId":"1425478229-11252-2-git-send-email-szeder@ira.uka.de","threadId":"38540","inReplyTo":"1425478229-11252-1-git-send-email-szeder@ira.uka.de","subject":"[PATCH 2/2] completion: simplify __git_remotes()","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-03-04T14:10:29Z","receivedAt":"2015-03-04T14:10:29Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The __git_remotes() helper function lists the remotes from the config\nfile by processing the output of a 'git config' query.  A simple 'git\nremote' produces the exact same output, so run that instead.\n\nRemotes under '$GIT_DIR/remotes' are still listed by running 'ls -1',\nbecause 'git remote' unfortunately ignores them.\n\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\n contrib/completion/git-completion.bash | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex c21190d..f5ae5e3 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -411,12 +411,9 @@ __git_refs_remotes ()\n \n __git_remotes ()\n {\n-\tlocal i IFS=$'\\n' d=\"$(__gitdir)\"\n+\tlocal d=\"$(__gitdir)\"\n \ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n-\tfor i in $(git --git-dir=\"$d\" config --get-regexp 'remote\\..*\\.url' 2>/dev/null); do\n-\t\ti=\"${i#remote.}\"\n-\t\techo \"${i/.url*/}\"\n-\tdone\n+\tgit --git-dir=\"$d\" remote\n }\n \n __git_list_merge_strategies ()\n-- \n2.1.2.1369.g8751039\n"}]}