Re: [PATCH] git-completion.bash: update obsolete code.
Manlio Perillo writes: > Il 17/12/2012 05:54, Junio C Hamano ha scritto: >> Manlio Perillo 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 >>> --- >> >> 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-completion.bash: update obsolete code.
Manlio Perillo 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-completion.bash: update obsolete code.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Il 17/12/2012 05:54, Junio C Hamano ha scritto: > Manlio Perillo 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 >> --- > > 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- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-completion.bash: update obsolete code.
Manlio Perillo 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
> ---
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,$, ,
> -}
> -/^12 blob /{
> -s,^.*,,
> -s,$, ,
> -}
> -/^04 tree /{
> -s,^.*,,
> -s,$,/,
> -}
> -s/^.*//')" \
> + __gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree --name-only
> "$ls")" \
> "$pfx" "$cur_" ""
> ;;
> *...*)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
