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

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Nov 14, 2012, 00:09 UTC
Message-ID: <20121114000909.GA14953@goldbirke>
In-Reply-To: <CAFj1UpFd9X8Jq5o7B4m35i=merBDvOo4NOtwth=UnG2S5X_rGw@mail.gmail.com>

Hi,

I've got two more comments.

On Mon, Nov 12, 2012 at 03:07:46PM -0500, Marc Khouzam wrote:

Show 14 quoted lines

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

Show 6 quoted lines

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