Re: Fwd: [PATCH] Add tcsh-completion support to contrib by using git-completion.bash

["SZEDER Gábor" <szeder@ira.uka.de>]

Hi,


I've got two more comments.

On Mon, Nov 12, 2012 at 03:07:46PM -0500, Marc Khouzam wrote:
> @@ -2481,3 +2483,52 @@ __git_complete gitk __gitk_main
>  if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
>  __git_complete git.exe __git_main
>  fi
> +
> +# Method that will output the result of the completion done by
> +# the bash completion script, so that it can be re-used in another
> +# context than the bash complete command.
> +# It accepts 1 to 2 arguments:
> +# 1: The command-line to complete
> +# 2: The index of the word within argument #1 in which the cursor is
> +#    located (optional). If parameter 2 is not provided, it will be
> +#    determined as best possible using parameter 1.
> +_git_complete_with_output ()

We differentiate between _git_whatever() and __git_whatever()
functions.  The former performs completion for the 'whatever' git
command/alias, the latter is a completion helper function.  This
is a helper function, so it should begin with double underscores.

> +{
> +       # Set COMP_WORDS to the command-line as bash would.
> +       COMP_WORDS=($1)
> +
> +       # Set COMP_CWORD to the cursor location as bash would.
> +       if [ -n "$2" ]; then

A while ago the completion script was made 'set -u'-clean.  (If 'set
-u' is enabled, then it's an error to access undefined variables).
I'm not sure how many people are out there who'd use this script for
tcsh while having 'set -u' in their profile...  probably not that
many.  Still, I think it would be great to keep it up.

Here $2 would be undefined, so accessingit it would cause an error
under those semantincs.  Please use ${2-} instead (use empty string
when undefined).

> +if [ -n "$1" ] ; then

Same here.

> +  # If there is an argument, we know the script is being executed
> +  # so go ahead and run the _git_complete_with_output function
> +  _git_complete_with_output "$1" "$2"

And here.

Thanks
Gábor