Re: Problem completing remotes when .git/remotes exits

2012-09-26 Thread SZEDER Gábor
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

2012-09-25 Thread SZEDER Gábor
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

2012-09-25 Thread Junio C Hamano
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

2012-09-25 Thread SZEDER Gábor
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

2012-09-25 Thread SZEDER Gábor
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