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

[Junio C Hamano <gitster@pobox.com>]

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

> Il 11/01/2013 23:02, Junio C Hamano ha scritto:
>> Manlio Perillo <manlio.perillo@gmail.com> writes:
>> 
>>> +# 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's not tricky: it is POSIX:

I know that.  It is an instance of "Even it is in POSIX, we may want
to refrain using it, because some shells get it wrong, and it is
easy to work it around".

>> effect during the execution of the { block } (in other words, it is
>> not about squelching errors during the function definition).
>
> What do you mean by "squelching"?

Silencing, not showing the end user.  Sending to /dev/null.

> I have added tcsh to the sh list, but it fails with:
> Badly placed ()'s.

tcsh (and csh) are not even in the Bourne shell family and is not
expected to be able to run any non trivial POSIX shell scripts.  The
completion script for it does not dot-source this but instead lets
bash read it, so it is fine.

>> 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).
>
> Ok, I was wrong on assuming all modern shells were POSIX compliant.

Shells in csh family will never be, and being non-POSIX is not a
crime.  It only matters when such a shell is allowed to dot-source
this script, and this script uses constructs that such a shell does
not understand.

> I will change the code to use a nested {} group.
>
>> 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?
>
> I don't know.

The caller of this function figures out the value of $subdirname,
and calls you; your "cd $subdirname" may not go to ./$subdirname as
you expect, but to the $subdirname directory under one of the
directories listed in CDPATH, before running ls-tree or ls-files.