* [PATCH] completion: avoid ls-remote in certain scenarios
@ 2013-05-29 3:20 Felipe Contreras
2013-05-29 7:44 ` SZEDER Gábor
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Felipe Contreras @ 2013-05-29 3:20 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
It's _very_ slow in many cases, and there's really no point in fetching
*everything* from the remote just for completion. In many cases it might
be faster for the user to type the whole thing.
If the user manually specifies 'refs/*', then the full ls-remote
completion is triggered.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
contrib/completion/git-completion.bash | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1c35eef..2ce4f7d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -427,14 +427,8 @@ __git_refs ()
done
;;
*)
- git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \
- while read -r hash i; do
- case "$i" in
- *^{}) ;;
- refs/*) echo "${i#refs/*/}" ;;
- *) echo "$i" ;;
- esac
- done
+ echo "HEAD"
+ git for-each-ref --format="%(refname:short)" -- "refs/remotes/$dir/" | sed -e "s#^$dir/##"
;;
esac
}
--
1.8.3.rc3.312.g47657de
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] completion: avoid ls-remote in certain scenarios
2013-05-29 3:20 [PATCH] completion: avoid ls-remote in certain scenarios Felipe Contreras
@ 2013-05-29 7:44 ` SZEDER Gábor
2013-06-01 13:58 ` Felipe Contreras
2013-06-01 18:25 ` Ramkumar Ramachandra
2013-06-02 22:08 ` Junio C Hamano
2 siblings, 1 reply; 6+ messages in thread
From: SZEDER Gábor @ 2013-05-29 7:44 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano
On Tue, May 28, 2013 at 10:20:48PM -0500, Felipe Contreras wrote:
> It's _very_ slow in many cases, and there's really no point in fetching
> *everything* from the remote just for completion. In many cases it might
> be faster for the user to type the whole thing.
>
> If the user manually specifies 'refs/*', then the full ls-remote
> completion is triggered.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> contrib/completion/git-completion.bash | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 1c35eef..2ce4f7d 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -427,14 +427,8 @@ __git_refs ()
> done
> ;;
> *)
> - git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \
> - while read -r hash i; do
> - case "$i" in
> - *^{}) ;;
> - refs/*) echo "${i#refs/*/}" ;;
> - *) echo "$i" ;;
> - esac
> - done
> + echo "HEAD"
> + git for-each-ref --format="%(refname:short)" -- "refs/remotes/$dir/" | sed -e "s#^$dir/##"
This case statement is only executed when $dir is not a git directory,
so what ensures that the cwd is in a git repo or work tree when
executing this brach of the case statement? What about 'git
--git-dir=/path/to/repo' invocations or when $GIT_DIR is specified?
Gábor
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] completion: avoid ls-remote in certain scenarios
2013-05-29 7:44 ` SZEDER Gábor
@ 2013-06-01 13:58 ` Felipe Contreras
0 siblings, 0 replies; 6+ messages in thread
From: Felipe Contreras @ 2013-06-01 13:58 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Junio C Hamano
On Wed, May 29, 2013 at 2:44 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> On Tue, May 28, 2013 at 10:20:48PM -0500, Felipe Contreras wrote:
>> It's _very_ slow in many cases, and there's really no point in fetching
>> *everything* from the remote just for completion. In many cases it might
>> be faster for the user to type the whole thing.
>>
>> If the user manually specifies 'refs/*', then the full ls-remote
>> completion is triggered.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>> contrib/completion/git-completion.bash | 10 ++--------
>> 1 file changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 1c35eef..2ce4f7d 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -427,14 +427,8 @@ __git_refs ()
>> done
>> ;;
>> *)
>> - git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \
>> - while read -r hash i; do
>> - case "$i" in
>> - *^{}) ;;
>> - refs/*) echo "${i#refs/*/}" ;;
>> - *) echo "$i" ;;
>> - esac
>> - done
>> + echo "HEAD"
>> + git for-each-ref --format="%(refname:short)" -- "refs/remotes/$dir/" | sed -e "s#^$dir/##"
>
> This case statement is only executed when $dir is not a git directory,
> so what ensures that the cwd is in a git repo or work tree when
> executing this brach of the case statement? What about 'git
> --git-dir=/path/to/repo' invocations or when $GIT_DIR is specified?
'git --git-dir=foo fetch <tab>' doesn't even work. I sent the patches
to fix it, but as usual, nobody cared about actual real fixes.
$GIT_DIR works fine, why wouldn't it?
--
Felipe Contreras
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] completion: avoid ls-remote in certain scenarios
2013-05-29 3:20 [PATCH] completion: avoid ls-remote in certain scenarios Felipe Contreras
2013-05-29 7:44 ` SZEDER Gábor
@ 2013-06-01 18:25 ` Ramkumar Ramachandra
2013-06-02 22:08 ` Junio C Hamano
2 siblings, 0 replies; 6+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-01 18:25 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano
Felipe Contreras wrote:
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 1c35eef..2ce4f7d 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -427,14 +427,8 @@ __git_refs ()
> done
> ;;
> *)
> - git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \
> - while read -r hash i; do
> - case "$i" in
> - *^{}) ;;
> - refs/*) echo "${i#refs/*/}" ;;
> - *) echo "$i" ;;
> - esac
> - done
> + echo "HEAD"
> + git for-each-ref --format="%(refname:short)" -- "refs/remotes/$dir/" | sed -e "s#^$dir/##"
Yeah, this patch makes sense overall. I'm curious as to what role
ORG_HEAD had to play originally, and why it's absent now.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] completion: avoid ls-remote in certain scenarios
2013-05-29 3:20 [PATCH] completion: avoid ls-remote in certain scenarios Felipe Contreras
2013-05-29 7:44 ` SZEDER Gábor
2013-06-01 18:25 ` Ramkumar Ramachandra
@ 2013-06-02 22:08 ` Junio C Hamano
2013-06-02 22:57 ` Felipe Contreras
2 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2013-06-02 22:08 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
Felipe Contreras <felipe.contreras@gmail.com> writes:
> It's _very_ slow in many cases, and there's really no point in fetching
> *everything* from the remote just for completion. In many cases it might
> be faster for the user to type the whole thing.
Besides, if I understand the use of __git_refs correctly, it is
primarily about completing things like
git checkout -b topic origin/<TAB>
where you actively want _local_ copies of what you currently have in
refs/remotes/origin/, not what you would get if you were to fetch
and then type the command again, so in that sense, using ls-remote
is a wrong thing to do in the first place.
This however may need to be made optional if this is also being used
to complete
git fetch git://g.k.org/pub/scm/git/git.git/ <TAB>
to list what can be fetched, but I do not think that is a very
common thing to do (you either know what you want to fetch, in which
case you do not complete but copy & paste, or more likely you have a
named remote and fetch the whole thing).
So I think overall the above makes sense.
> If the user manually specifies 'refs/*', then the full ls-remote
> completion is triggered.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> contrib/completion/git-completion.bash | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 1c35eef..2ce4f7d 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -427,14 +427,8 @@ __git_refs ()
> done
> ;;
> *)
> - git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \
> - while read -r hash i; do
> - case "$i" in
> - *^{}) ;;
> - refs/*) echo "${i#refs/*/}" ;;
> - *) echo "$i" ;;
> - esac
> - done
> + echo "HEAD"
> + git for-each-ref --format="%(refname:short)" -- "refs/remotes/$dir/" | sed -e "s#^$dir/##"
> ;;
> esac
> }
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] completion: avoid ls-remote in certain scenarios
2013-06-02 22:08 ` Junio C Hamano
@ 2013-06-02 22:57 ` Felipe Contreras
0 siblings, 0 replies; 6+ messages in thread
From: Felipe Contreras @ 2013-06-02 22:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Sun, Jun 2, 2013 at 5:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> It's _very_ slow in many cases, and there's really no point in fetching
>> *everything* from the remote just for completion. In many cases it might
>> be faster for the user to type the whole thing.
>
> Besides, if I understand the use of __git_refs correctly, it is
> primarily about completing things like
>
> git checkout -b topic origin/<TAB>
>
> where you actively want _local_ copies of what you currently have in
> refs/remotes/origin/, not what you would get if you were to fetch
> and then type the command again, so in that sense, using ls-remote
> is a wrong thing to do in the first place.
Yeah, but we wouldn't use ls-remote in those cases, only when __gitdir
is not available. Try 'git checkout <TAB>' outside a git repository.
> This however may need to be made optional if this is also being used
> to complete
>
> git fetch git://g.k.org/pub/scm/git/git.git/ <TAB>
>
> to list what can be fetched, but I do not think that is a very
> common thing to do (you either know what you want to fetch, in which
> case you do not complete but copy & paste, or more likely you have a
> named remote and fetch the whole thing).
Indeed. And it doesn't make sense to punish the typical use-cases
because of the atypical ones.
Besides:
git fetch git://g.k.org/pub/scm/git/git.git/ refs/<TAB>
Would still complete everything.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-02 22:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-29 3:20 [PATCH] completion: avoid ls-remote in certain scenarios Felipe Contreras
2013-05-29 7:44 ` SZEDER Gábor
2013-06-01 13:58 ` Felipe Contreras
2013-06-01 18:25 ` Ramkumar Ramachandra
2013-06-02 22:08 ` Junio C Hamano
2013-06-02 22:57 ` Felipe Contreras
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).