* Problem completing remotes when .git/remotes exits
@ 2012-09-19 19:55 Johannes Sixt
2012-09-25 23:00 ` SZEDER Gábor
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Sixt @ 2012-09-19 19:55 UTC (permalink / raw)
To: Git Mailing List
I have an empty .git/remotes directory. Trying to complete the name of
a remote always reports an error:
git@master:1023> git fetch <TAB>ls: 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?
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index be800e0..824f5b6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -369,8 +369,9 @@ __git_refs_remotes ()
__git_remotes ()
{
- local i IFS=$'\n' d="$(__gitdir)"
+ local i d="$(__gitdir)"
test -d "$d/remotes" && ls -1 "$d/remotes"
+ local IFS=$'\n'
for i in $(git --git-dir="$d" config --get-regexp 'remote\..*\.url' 2>/dev/null); do
i="${i#remote.}"
echo "${i/.url*/}"
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Problem completing remotes when .git/remotes exits
2012-09-19 19:55 Problem completing remotes when .git/remotes exits Johannes Sixt
@ 2012-09-25 23:00 ` SZEDER Gábor
2012-09-25 23:43 ` Junio C Hamano
2012-09-26 1:09 ` SZEDER Gábor
0 siblings, 2 replies; 6+ messages in thread
From: SZEDER Gábor @ 2012-09-25 23:00 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git Mailing List
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 <TAB>ls: 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Problem completing remotes when .git/remotes exits
2012-09-25 23:00 ` SZEDER Gábor
@ 2012-09-25 23:43 ` Junio C Hamano
2012-09-26 11:43 ` SZEDER Gábor
2012-09-26 1:09 ` SZEDER Gábor
1 sibling, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2012-09-25 23:43 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Johannes Sixt, Git Mailing List
SZEDER Gábor <szeder@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-<.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Problem completing remotes when .git/remotes exits
2012-09-25 23:00 ` SZEDER Gábor
2012-09-25 23:43 ` Junio C Hamano
@ 2012-09-26 1:09 ` SZEDER Gábor
2012-09-26 1:26 ` SZEDER Gábor, Johannes Sixt
1 sibling, 1 reply; 6+ messages in thread
From: SZEDER Gábor @ 2012-09-26 1:09 UTC (permalink / raw)
To: Git Mailing List; +Cc: Johannes Sixt
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Problem completing remotes when .git/remotes exits
2012-09-26 1:09 ` SZEDER Gábor
@ 2012-09-26 1:26 ` SZEDER Gábor, Johannes Sixt
0 siblings, 0 replies; 6+ messages in thread
From: SZEDER Gábor, Johannes Sixt @ 2012-09-26 1:26 UTC (permalink / raw)
To: Git Mailing List
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Problem completing remotes when .git/remotes exits
2012-09-25 23:43 ` Junio C Hamano
@ 2012-09-26 11:43 ` SZEDER Gábor
0 siblings, 0 replies; 6+ messages in thread
From: SZEDER Gábor @ 2012-09-26 11:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Git Mailing List
On Tue, Sep 25, 2012 at 04:43:59PM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder@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
$
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-26 11:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-19 19:55 Problem completing remotes when .git/remotes exits Johannes Sixt
2012-09-25 23:00 ` SZEDER Gábor
2012-09-25 23:43 ` Junio C Hamano
2012-09-26 11:43 ` SZEDER Gábor
2012-09-26 1:09 ` SZEDER Gábor
2012-09-26 1:26 ` SZEDER Gábor, Johannes Sixt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.