{"thread":{"id":"31611","subject":"Problem completing remotes when .git/remotes exits","startedAt":"2012-09-19T19:55:28Z","lastAt":"2012-09-26T11:43:15Z","messageCount":6,"participants":["Johannes Sixt","SZEDER Gábor","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"199523","messageId":"505A2330.9040800@kdbg.org","threadId":"31611","inReplyTo":null,"subject":"Problem completing remotes when .git/remotes exits","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-09-19T19:55:28Z","receivedAt":"2012-09-19T19:55:28Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"I have an empty .git/remotes directory. Trying to complete the name of\na remote always reports an error:\n\ngit@master:1023> git fetch <TAB>ls: invalid option -- ' '\nTry `ls --help' for more information.\n\nI have these:\n\n\talias ls='ls $LS_OPTIONS'\nand\n\tLS_OPTIONS='-N --color=tty -T 0'\n\nI instrumented __git_remotes with set -x, which shows:\n\ngit@master:1006> git fetch <TAB>+++ __gitdir\n+++ '[' -z '' ']'\n+++ '[' -n '' ']'\n+++ '[' -n '' ']'\n+++ '[' -d .git ']'\n+++ echo .git\n++ local i 'IFS=\n' d=.git\n++ test -d .git/remotes\n++ ls '-N --color=tty -T 0' -1 .git/remotes\nls: invalid option -- ' '\nTry `ls --help' for more information.\n...\n\nNotice that the expansion of $LS_OPTIONS is not split at the blanks,\nobviously, because $IFS does not contain a blank at that moment.\n\nThe patch below helps, but it looks like a work-around rather than a\nsolution. Ideas?\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex be800e0..824f5b6 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -369,8 +369,9 @@ __git_refs_remotes ()\n \n __git_remotes ()\n {\n-\tlocal i IFS=$'\\n' d=\"$(__gitdir)\"\n+\tlocal i d=\"$(__gitdir)\"\n \ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n+\tlocal IFS=$'\\n'\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":"199912","messageId":"20120925230045.GA13266@goldbirke","threadId":"31611","inReplyTo":"505A2330.9040800@kdbg.org","subject":"Re: Problem completing remotes when .git/remotes exits","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-09-25T23:00:45Z","receivedAt":"2012-09-25T23:00:45Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n\nOn Wed, Sep 19, 2012 at 09:55:28PM +0200, Johannes Sixt wrote:\n> I have an empty .git/remotes directory. Trying to complete the name of\n> a remote always reports an error:\n> \n> git@master:1023> git fetch <TAB>ls: invalid option -- ' '\n> Try `ls --help' for more information.\n> \n> I have these:\n> \n> \talias ls='ls $LS_OPTIONS'\n> and\n> \tLS_OPTIONS='-N --color=tty -T 0'\n> \n> I instrumented __git_remotes with set -x, which shows:\n> \n> git@master:1006> git fetch <TAB>+++ __gitdir\n> +++ '[' -z '' ']'\n> +++ '[' -n '' ']'\n> +++ '[' -n '' ']'\n> +++ '[' -d .git ']'\n> +++ echo .git\n> ++ local i 'IFS=\n> ' d=.git\n> ++ test -d .git/remotes\n> ++ ls '-N --color=tty -T 0' -1 .git/remotes\n> ls: invalid option -- ' '\n> Try `ls --help' for more information.\n> ...\n> \n> Notice that the expansion of $LS_OPTIONS is not split at the blanks,\n> obviously, because $IFS does not contain a blank at that moment.\n> \n> The patch below helps, but it looks like a work-around rather than a\n> solution. Ideas?\n\nI've got two alternative solutions for this issue.\n\nThe first one is less intrusive: use the 'command' builtin to tell\nthe shell to ignore shell functions and aliases and just run the ls\ncommand.\n\ndiff --git a/contrib/completion/git-completion.bash\nb/contrib/completion/git-completion.bash\nindex be800e09..bcde9472 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -370,7 +370,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\" && command 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\n\nBut then it got me thinking...  Notice how much effort we spend just\nto get the list of remotes?  We could just run 'git remote' directly\ninstead...\n\ndiff --git a/contrib/completion/git-completion.bash\nb/contrib/completion/git-completion.bash\nindex be800e09..1daeaccf 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -369,12 +369,8 @@ __git_refs_remotes ()\n \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+\tlocal d=\"$(__gitdir)\"\n+\tgit --git-dir=\"$d\" remote\n }\n \n __git_list_merge_strategies ()\n\n\nI prefer the second one ;)\n\nBest,\nGábor\n"},{"id":"199917","messageId":"7vbogtvfb4.fsf@alter.siamese.dyndns.org","threadId":"31611","inReplyTo":"20120925230045.GA13266@goldbirke","subject":"Re: Problem completing remotes when .git/remotes exits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-25T23:43:59Z","receivedAt":"2012-09-25T23:43:59Z","isPatch":false,"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> -\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n> +\ttest -d \"$d/remotes\" && command ls -1 \"$d/remotes\"\n\nYuck.  For normal scripts, nobody sane would define \"alias\" for\nnon-interactive environments, but because these things work in an\ninteractive environment, we have to protect ourselves from user\naliases.  Not just \"ls\", but \"test\" we see above may misbehave X-<.\n"},{"id":"199920","messageId":"20120926010938.GD10144@goldbirke","threadId":"31611","inReplyTo":"20120925230045.GA13266@goldbirke","subject":"Re: Problem completing remotes when .git/remotes exits","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-09-26T01:09:38Z","receivedAt":"2012-09-26T01:09:38Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Sep 26, 2012 at 01:00:45AM +0200, SZEDER Gábor wrote:\n> But then it got me thinking...  Notice how much effort we spend just\n> to get the list of remotes?  We could just run 'git remote' directly\n> instead...\n\nActually, we can't, because 'git remote' doesn't seem to list remotes\nstored under .git/remotes.  Is that intentional?\n\nAnyway, we could still use 'git remote' to replace at least the config\nquery and the for loop to spare a few lines of code and a subshell.\n"},{"id":"199921","messageId":"20120926012633.GE10144@goldbirke","threadId":"31611","inReplyTo":"20120926010938.GD10144@goldbirke","subject":"Re: Problem completing remotes when .git/remotes exits","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-09-26T01:26:33Z","receivedAt":"2012-09-26T01:26:33Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Sep 26, 2012 at 03:09:38AM +0200, SZEDER Gábor wrote:\n> On Wed, Sep 26, 2012 at 01:00:45AM +0200, SZEDER Gábor wrote:\n> > But then it got me thinking...  Notice how much effort we spend just\n> > to get the list of remotes?  We could just run 'git remote' directly\n> > instead...\n> \n> Actually, we can't, because 'git remote' doesn't seem to list remotes\n> stored under .git/remotes.  Is that intentional?\n\nLooks like a bug, as it seems to have been lost in translation in\n211c8968 (Make git-remote a builtin, 2008-02-29).\n\nlist_remote() in git-remote.perl looks for remotes in config and in\n.git/remotes/, too.  The builtin implementation uses\nremote.c:for_each_remote() from the start, which only looks at the\nconfig.\n"},{"id":"199943","messageId":"20120926114315.GF10144@goldbirke","threadId":"31611","inReplyTo":"7vbogtvfb4.fsf@alter.siamese.dyndns.org","subject":"Re: Problem completing remotes when .git/remotes exits","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-09-26T11:43:15Z","receivedAt":"2012-09-26T11:43:15Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Sep 25, 2012 at 04:43:59PM -0700, Junio C Hamano wrote:\n> SZEDER Gábor <szeder@ira.uka.de> writes:\n> \n> > -\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n> > +\ttest -d \"$d/remotes\" && command ls -1 \"$d/remotes\"\n> \n> Yuck.  For normal scripts, nobody sane would define \"alias\" for\n> non-interactive environments, but because these things work in an\n> interactive environment, we have to protect ourselves from user\n> aliases.  Not just \"ls\", but \"test\" we see above may misbehave X-<.\n\nRight, however, while ls is frequently aliased (my ubuntu box has\nalias ls='ls --color=auto' in /etc/skel/.bashrc by default, but that's\nnot an issue in this case), I think aliasing test is just crazy.\nYeah, it's possible, but if we go down that route, then we should also\nworry about the [ builtin being aliased:\n\n$ if [ -r nonexisting ] ; then echo found ; fi\n$ alias [='echo using aliased ['\n$ if [ -r nonexisting ] ; then echo found ; fi\nusing aliased [ -r nonexisting ]\nfound\n$\n"}]}