* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).