Re: [PATCH v5] git-completion.bash: add support for path completion

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 11, 2013, 22:02 UTC
Message-ID: <7v8v7zbcoi.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1357930123-26310-1-git-send-email-manlio.perillo@gmail.com>

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

Show 17 quoted lines

> +# Process path list returned by "ls-files" and "diff-index --name-only"
> +# commands, in order to list only file names relative to a specified
> +# directory, and append a slash to directory names.
> +__git_index_file_list_filter ()
> +{
> +	# Default to Bash >= 4.x
> +	__git_index_file_list_filter_bash
> +}
> +
> +# Execute git ls-files, returning paths relative to the directory
> +# specified in the first argument, and using the options specified in
> +# the second argument.
> +__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

I think this redirection is correct but a bit tricky; it is in effect during the execution of the { block } (in other words, it is not about squelching errors during the function definition).

-- >8 --
#!/bin/sh
cat >t.sh <<\EOF &&
echo I am "$1"
t () { echo "Goes to stdout"; echo >&2 "Goes to stderr"; } 2>/dev/null
t
for sh in bash dash ksh zsh
do
	$sh t.sh $sh
done
-- 8< --

Bash does (so do dash and real AT&T ksh) grok this correctly, but zsh does not seem to (I tried zsh 4.3.10 and 4.3.17; also zsh pretending to be ksh gets this wrong as well). Not that what ksh does matters, as it won't be dot-sourcing bash completion script.

It however may affect zsh, which does seem to dot-source this file. Perhaps zsh completion may have to be rewritten in a similar way as tcsh completion is done (i.e. does not dot-source this file but ask bash to do the heavy-lifting).

This function seems to be always called in an subshell (e.g. as an upstream of a pipeline), so the "cd" may be harmless, but don't you need to disable CDPATH while doing this?

Show 7 quoted lines

> +# Execute git diff-index, returning paths relative to the directory
> +# specified in the first argument, and using the tree object id
> +# specified in the second argument.
> +__git_diff_index_helper ()
> +{
> +	cd "$1" && git diff-index --name-only --relative "$2"
> +} 2>/dev/null

Ditto.

Show 11 quoted lines

> @@ -722,6 +875,43 @@ __git_has_doubledash ()
>  	return 1
>  }
>  
> +# Try to count non option arguments passed on the command line for the
> +# specified git command.
> +# When options are used, it is necessary to use the special -- option to
> +# tell the implementation were non option arguments begin.
> +# XXX this can not be improved, since options can appear everywhere, as
> +# an example:
> +#	git mv x -n y

If that is the case, it is a bug in the command line parser, I think. We should reject it, and the command line completer certainly should not encourage it.