Re: [RFC/PATCH] git-completion.bash: remove bashism to fix ZSH compatibility

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 11, 2013, 16:17 UTC
Message-ID: <7v38w1c3ms.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1363004487-1193-1-git-send-email-Matthieu.Moy@imag.fr>

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

Show 8 quoted lines

> The function-wide redirection used for __git_ls_files_helper and
> __git_diff_index_helper work only with bash. Using ZSH, trying to
> complete an inexistant directory gave this:
>
>   git add no-such-dir/__git_ls_files_helper:cd:2: no such file or directory: no-such-dir/
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---

This is not bash-ism but POSIX.1, even though it is not very well known. I recall commenting on this exact pattern during the review.

  http://thread.gmane.org/gmane.comp.version-control.git/213232/focus=213286

After all, I was right when I said that some implementations may get it wrong and we shouldn't use the construct X-<.

> These two instances seem to be the only ones in the file.
>
> I'm not sure whether the 2>/dev/null would be needed for the command
> on the RHS of the && too (git ls-files and git diff-index).

It would not hurt to discard their standard error.

Show 29 quoted lines

>  contrib/completion/git-completion.bash | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index b62bec0..0640274 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -300,8 +300,8 @@ __git_index_file_list_filter ()
>  __git_ls_files_helper ()
>  {
>  	# NOTE: $2 is not quoted in order to support multiple options
> -	cd "$1" && git ls-files --exclude-standard $2
> -} 2>/dev/null
> +	cd "$1" 2>/dev/null && git ls-files --exclude-standard $2
> +}
>  
>  
>  # Execute git diff-index, returning paths relative to the directory
> @@ -309,8 +309,8 @@ __git_ls_files_helper ()
>  # specified in the second argument.
>  __git_diff_index_helper ()
>  {
> -	cd "$1" && git diff-index --name-only --relative "$2"
> -} 2>/dev/null
> +	cd "$1" 2>/dev/null && git diff-index --name-only --relative "$2"
> +}
>  
>  # __git_index_files accepts 1 or 2 arguments:
>  # 1: Options to pass to ls-files (required).