[PATCH] git-completion.bash: update obsolete code.

[PATCH] git-completion.bash: update obsolete code.

[Manlio Perillo]

The git-completion.bash script was using the git ls-tree command
without the --name-only option, with a sed filter to parse path names;
use the --name-only option, instead.

Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
---
 contrib/completion/git-completion.bash | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0b77eb1..85d9051 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -397,20 +397,7 @@ __git_complete_revlist_file ()
 		*)   pfx="$ref:$pfx" ;;
 		esac
 
-		__gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree "$ls" \
-				| sed '/^100... blob /{
-				           s,^.*	,,
-				           s,$, ,
-				       }
-				       /^120000 blob /{
-				           s,^.*	,,
-				           s,$, ,
-				       }
-				       /^040000 tree /{
-				           s,^.*	,,
-				           s,$,/,
-				       }
-				       s/^.*	//')" \
+		__gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree --name-only "$ls")" \
 			"$pfx" "$cur_" ""
 		;;
 	*...*)
-- 
1.8.1.rc1.18.g9db0d25

Re: [PATCH] git-completion.bash: update obsolete code.

[Junio C Hamano]

Manlio Perillo <manlio.perillo@gmail.com> writes:

> The git-completion.bash script was using the git ls-tree command
> without the --name-only option, with a sed filter to parse path names;
> use the --name-only option, instead.
>
> Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
> ---

Did you miss the different handling between blobs and trees the
latter gets trailing slash in the completion)?

>  contrib/completion/git-completion.bash | 15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 0b77eb1..85d9051 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -397,20 +397,7 @@ __git_complete_revlist_file ()
>  		*)   pfx="$ref:$pfx" ;;
>  		esac
>  
> -		__gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree "$ls" \
> -				| sed '/^100... blob /{
> -				           s,^.*	,,
> -				           s,$, ,
> -				       }
> -				       /^120000 blob /{
> -				           s,^.*	,,
> -				           s,$, ,
> -				       }
> -				       /^040000 tree /{
> -				           s,^.*	,,
> -				           s,$,/,
> -				       }
> -				       s/^.*	//')" \
> +		__gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree --name-only "$ls")" \
>  			"$pfx" "$cur_" ""
>  		;;
>  	*...*)

Re: [PATCH] git-completion.bash: update obsolete code.

[Manlio Perillo]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 17/12/2012 05:54, Junio C Hamano ha scritto:
> Manlio Perillo <manlio.perillo@gmail.com> writes:
> 
>> The git-completion.bash script was using the git ls-tree command
>> without the --name-only option, with a sed filter to parse path names;
>> use the --name-only option, instead.
>>
>> Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
>> ---
> 
> Did you miss the different handling between blobs and trees the
> latter gets trailing slash in the completion)?
> 

Yes, I totally missed it, sorry.
I was assuming the bash completion script was written before --name-only
option was added to ls-tree.

By the way, IMHO there should be an option for adding a slash to
directory names in ls-tree.

> [...]


Regards  Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEUEARECAAYFAlDPTgUACgkQscQJ24LbaUSkKgCePH2NFHf/qp2ZrgI9eD9D0D3G
zOwAmL8Dc8r9DevyV1ZhaCb2G9MPZxU=
=QJEy
-----END PGP SIGNATURE-----

Re: [PATCH] git-completion.bash: update obsolete code.

[Junio C Hamano]

Manlio Perillo <manlio.perillo@gmail.com> writes:

> By the way, IMHO there should be an option for adding a slash to
> directory names in ls-tree.

I am not sure about that; ls-tree is meant to be used by scripts
that are capable of doing that kind of thing themselves.

If we were to add an option to add a slash, I think it should be
modeled after "ls -F", whose output is meant for humans and no sane
scripts would read from it.  It is very likely that such an option
should add @ at the end of the name for a symbolic link, so it won't
be useful for your patch.

Re: [PATCH] git-completion.bash: update obsolete code.

[Junio C Hamano]

Manlio Perillo <manlio.perillo@gmail.com> writes:

> Il 17/12/2012 05:54, Junio C Hamano ha scritto:
>> Manlio Perillo <manlio.perillo@gmail.com> writes:
>> 
>>> The git-completion.bash script was using the git ls-tree command
>>> without the --name-only option, with a sed filter to parse path names;
>>> use the --name-only option, instead.
>>>
>>> Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
>>> ---
>> 
>> Did you miss the different handling between blobs and trees the
>> latter gets trailing slash in the completion)?
>> 
>
> Yes, I totally missed it, sorry.
> I was assuming the bash completion script was written before --name-only
> option was added to ls-tree.

There is no need to say "sorry" here; catching this kind of mistake
is what we have the patch review process for, after all.  If
anything, "thanks" is more appropriate ;-)

And the way you stated the reasoning behind this change in the
proposed commit log message was really good.  It showed clearly that
the patch was an attempt to clean up the code, not was an attempt to
change the behaviour to drop the trailing slash or space.  If it
said "let's use ls-tree --name-only" without such an explanation on
"why", future readers of "git log" would have been left scratching
their heads, wondering "why" the change was made.

Thanks.