Re: [PATCH] Revert "completion: fix shell expansion of items"

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 25, 2012, 16:03 UTC
Message-ID: <7vpq5aw0m5.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120925043119.GA6208@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 6 quoted lines

> On Thu, Sep 20, 2012 at 08:11:52PM +0200, SZEDER Gábor wrote:
>
> Yeah, doing "git checkout jk/<tab>" is not working at all, and I noticed
> the buggy commit is on the maint track, but has not yet been released.
> I'm not sure of the solution, but I think we should do this in the
> meantime:

Thanks. Reverting is the right solution; it will leave a note in the history why a seemingly simple patch was a bad idea to remind us.

It is sad that existing test did not cover something fundamental like this, though. Anybody who's worked on completion wants to make the test coverage better?

Show 46 quoted lines

> -- >8 --
> Subject: Revert "completion: fix shell expansion of items"
>
> This reverts commit 25ae7cfd19c8f21721363c64163cd5d9d1135b20.
>
> That patch does fix expansion of weird variables in some
> simple tests, but it also seems to break other things, like
> expansion of refs by "git checkout".
>
> While we're sorting out the correct solution, we are much
> better with the original bug (people with metacharacters in
> their completions occasionally see an error message) than
> the current bug (ref completion does not work at all).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  contrib/completion/git-completion.bash | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 5a5b5a0..d743e56 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -225,13 +225,6 @@ fi
>  fi
>  fi
>  
> -# Quotes the argument for shell reuse
> -__git_quote()
> -{
> -	local quoted=${1//\'/\'\\\'\'}
> -	printf "'%s'" "$quoted"
> -}
> -
>  # Generates completion reply with compgen, appending a space to possible
>  # completion words, if necessary.
>  # It accepts 1 to 4 arguments:
> @@ -268,7 +261,7 @@ __gitcomp_nl ()
>  __gitcomp_nl ()
>  {
>  	local IFS=$'\n'
> -	COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$(__git_quote "$1")" -- "${3-$cur}"))
> +	COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
>  }
>  
>  __git_heads ()