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

From: Manlio Perillo <[email-masked]>
Date: Sat, 12 Jan 2013 15:52:56 +0100

Il 11/01/2013 23:02, Junio C Hamano ha scritto:
> Manlio Perillo <[email-masked]> 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:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_10

> it is in
> 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"?

Note that I originally wrote the code as

__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
}

but then I checked the POSIX standard, noting that it is redundant.

> -- >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< --
> 

There is a missing EOF delimiter.
And I'm not sure to understand the meaning of && after EOF.

> 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.
>

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

> 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.
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.

> [..]

>> +# 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.
> 

$ mkdir y
$ git mv x -n y
Checking rename of 'x' to 'y/x'
Renaming x to y/x
$ git status
# On branch master
nothing to commit, working directory clean

I was assuming it to be "normal", given how complex Git command line
parsing is (IMHO).

Thanks  Manlio