git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [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

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).