Re: Problem completing remotes when .git/remotes exits
On Tue, Sep 25, 2012 at 04:43:59PM -0700, Junio C Hamano wrote: SZEDER Gábor sze...@ira.uka.de writes: - test -d $d/remotes ls -1 $d/remotes + test -d $d/remotes command ls -1 $d/remotes Yuck. For normal scripts, nobody sane would define alias for non-interactive environments, but because these things work in an interactive environment, we have to protect ourselves from user aliases. Not just ls, but test we see above may misbehave X-. Right, however, while ls is frequently aliased (my ubuntu box has alias ls='ls --color=auto' in /etc/skel/.bashrc by default, but that's not an issue in this case), I think aliasing test is just crazy. Yeah, it's possible, but if we go down that route, then we should also worry about the [ builtin being aliased: $ if [ -r nonexisting ] ; then echo found ; fi $ alias [='echo using aliased [' $ if [ -r nonexisting ] ; then echo found ; fi using aliased [ -r nonexisting ] found $ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problem completing remotes when .git/remotes exits
Hi, On Wed, Sep 19, 2012 at 09:55:28PM +0200, Johannes Sixt wrote: I have an empty .git/remotes directory. Trying to complete the name of a remote always reports an error: git@master:1023 git fetch TABls: invalid option -- ' ' Try `ls --help' for more information. I have these: alias ls='ls $LS_OPTIONS' and LS_OPTIONS='-N --color=tty -T 0' I instrumented __git_remotes with set -x, which shows: git@master:1006 git fetch TAB+++ __gitdir +++ '[' -z '' ']' +++ '[' -n '' ']' +++ '[' -n '' ']' +++ '[' -d .git ']' +++ echo .git ++ local i 'IFS= ' d=.git ++ test -d .git/remotes ++ ls '-N --color=tty -T 0' -1 .git/remotes ls: invalid option -- ' ' Try `ls --help' for more information. ... Notice that the expansion of $LS_OPTIONS is not split at the blanks, obviously, because $IFS does not contain a blank at that moment. The patch below helps, but it looks like a work-around rather than a solution. Ideas? I've got two alternative solutions for this issue. The first one is less intrusive: use the 'command' builtin to tell the shell to ignore shell functions and aliases and just run the ls command. diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index be800e09..bcde9472 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -370,7 +370,7 @@ __git_refs_remotes () __git_remotes () { local i IFS=$'\n' d=$(__gitdir) - test -d $d/remotes ls -1 $d/remotes + test -d $d/remotes command ls -1 $d/remotes for i in $(git --git-dir=$d config --get-regexp 'remote\..*\.url' 2/dev/null); do i=${i#remote.} echo ${i/.url*/} But then it got me thinking... Notice how much effort we spend just to get the list of remotes? We could just run 'git remote' directly instead... diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index be800e09..1daeaccf 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -369,12 +369,8 @@ __git_refs_remotes () __git_remotes () { - local i IFS=$'\n' d=$(__gitdir) - test -d $d/remotes ls -1 $d/remotes - for i in $(git --git-dir=$d config --get-regexp 'remote\..*\.url' 2/dev/null); do - i=${i#remote.} - echo ${i/.url*/} - done + local d=$(__gitdir) + git --git-dir=$d remote } __git_list_merge_strategies () I prefer the second one ;) Best, Gábor -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problem completing remotes when .git/remotes exits
SZEDER Gábor sze...@ira.uka.de writes: - test -d $d/remotes ls -1 $d/remotes + test -d $d/remotes command ls -1 $d/remotes Yuck. For normal scripts, nobody sane would define alias for non-interactive environments, but because these things work in an interactive environment, we have to protect ourselves from user aliases. Not just ls, but test we see above may misbehave X-. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problem completing remotes when .git/remotes exits
On Wed, Sep 26, 2012 at 01:00:45AM +0200, SZEDER Gábor wrote: But then it got me thinking... Notice how much effort we spend just to get the list of remotes? We could just run 'git remote' directly instead... Actually, we can't, because 'git remote' doesn't seem to list remotes stored under .git/remotes. Is that intentional? Anyway, we could still use 'git remote' to replace at least the config query and the for loop to spare a few lines of code and a subshell. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problem completing remotes when .git/remotes exits
On Wed, Sep 26, 2012 at 03:09:38AM +0200, SZEDER Gábor wrote: On Wed, Sep 26, 2012 at 01:00:45AM +0200, SZEDER Gábor wrote: But then it got me thinking... Notice how much effort we spend just to get the list of remotes? We could just run 'git remote' directly instead... Actually, we can't, because 'git remote' doesn't seem to list remotes stored under .git/remotes. Is that intentional? Looks like a bug, as it seems to have been lost in translation in 211c8968 (Make git-remote a builtin, 2008-02-29). list_remote() in git-remote.perl looks for remotes in config and in .git/remotes/, too. The builtin implementation uses remote.c:for_each_remote() from the start, which only looks at the config. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html