From: John Keeping <john@keeping.me.uk>
Date: Sun, 08 Apr 2012 10:22:41 GMT
Subject: Re: [PATCH 05/12] completion: add missing global options
Message-ID: <20120408102230.GA22648@river.lan>
In-Reply-To: <1333854479-23260-6-git-send-email-felipe.contreras@gmail.com>

On Sun, Apr 08, 2012 at 06:07:52AM +0300, Felipe Contreras wrote:
> @@ -2621,7 +2621,10 @@ _git ()
>  		case "$i" in
>  		--git-dir=*) __git_dir="${i#--git-dir=}" ;;
>  		--bare)      __git_dir="." ;;
> -		--version|-p|--paginate) ;;
> +		--version) ;;
> +		-p|--paginate|--no-pager) ;;
> +		--exec-path=*|--html-path|--info-path) ;;

Since exec-path's argument is optional, shouldn't this be:

  +		--exec-path*|--html-path|--info-path) ;;

> +		--work-tree=*|--namespace=*|--no-replace-objects) ;;

It would be nice to accept options with argument with and without the
equals sign, as the git command does, so both:
    git --git-dir=/foo/bar
and
    git --git-dir /foo/bar

I guess this looks like (adding in -c as well):

		--git-dir)  ((c++)) ; __git_dir="${words[c]}" ;;
                --work-tree|--namespacei|-c) ((c++)) ;;


>  		--help) command="help"; break ;;
>  		*) command="$i"; break ;;
>  		esac
> @@ -2636,10 +2639,12 @@ _git ()
>  			--git-dir=
>  			--bare
>  			--version
> -			--exec-path
> +			--exec-path=

I think this change is incorrect since giving --exec-path without an
argument is perfectly fine and has a defined meaning.

>  			--html-path
> +			--info-path
>  			--work-tree=
>  			--namespace=
> +			--no-replace-objects
>  			--help

Should "-c" be in this list as well?

>  			"
>  			;;

-- 
John