Hi,
Quoting SZEDER Gábor <szeder@xxxxxxxxxx>:
Hi,
Quoting Junio C Hamano <gitster@xxxxxxxxx>:
SZEDER Gábor <szeder@xxxxxxxxxx> writes:
@@ -412,7 +412,7 @@ __git_refs_remotes ()
__git_remotes ()
{
local i IFS=$'\n' d="$(__gitdir)"
- test -d "$d/remotes" && ls -1 "$d/remotes"
+ test -d "$d/remotes" && ls -1 "$d/remotes" 2>/dev/null
for i in $(git --git-dir="$d" config --get-regexp
'remote\..*\.url' 2>/dev/null); do
i="${i#remote.}"
echo "${i/.url*/}"
Do I smell some bitrotting here?
This function just lists all the defined remotes, first by listing the
directories under refs/remotes to get the "legacy" remotes and then
loops over 'git config's output to get the "modern" ones. This
predates the arrival of the 'git remote' command in January 2007, so
it was really a long time ago.
We should just run 'git remote' instead, shouldn't we?
Perhaps. Is it sufficient to just make __git_remotes() a thin
wrapper around, i.e.
__git_remotes ()
{
git remotes
}
or do we need to munge its output further (I didn't look)?
Well, just like in other cases where we run git from the completion
script, we need a '--git-dir="$(__gitdir)"' as well, because the user can
specify the path to a different repo via $GIT_DIR or on the command
line.
Other than that it seems we are OK. Docs say "With no arguments,
shows a list of existing remotes." and as far as I can tell, on
MSysGit, it does so without any funny formatting.
Oh, look what forgotten treasure did I stumble upon in the vaults:
https://github.com/szeder/git/commit/e4e3760c15b485b9ff4768e13050f4b19b5968b8
A two and a half year old commit in my old git repo doing the same...
completely forgotten :)
Unfortunately, however, it's not quite that simple, because 'git
remote' doesn't list remotes under '$GIT_DIR/remotes'. Or at least I
would have expected the following test to work, but it does not:
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 17c6330..6a4c139 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -734,6 +734,15 @@ Pull: refs/heads/master:refs/heads/origin
Pull: refs/heads/next:refs/heads/origin2
EOF
+test_expect_success 'list remote in $GIT_DIR/remotes' '
+ mkdir .git/remotes &&
+ test_when_finished "rm -rf .git/remotes" &&
+ cat remotes_origin >.git/remotes/remote_from_file &&
+ git remote >actual &&
+ echo remote_from_file >expect &&
+ test_cmp expect actual
+'
+
test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' '
git clone one five &&
origin_url=$(pwd)/one &&
because listing remotes is implemented by for_each_remote(), which
only reads remotes from the config file.
Now, considering how old 'git remote' is, there were plenty of time
for someone to miss this functionality and complain about it, but
since it's still not implemented is probably a good sign that noone
has actually missed it. And I don't think it's worth implementing it
now just to shave off two more lines from the completion script.
Anyway, 'git remote' could still replace that 'git config' query. I
have the patches ready and it seems I got send-email working, so
they'll follow in a minute or two.
Best,
Gábor
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html