Re: [PATCH 3/3] completion: improve shell expansion of items

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Sep 28, 2012, 10:05 UTC
Message-ID: <20120928100530.GL10144@goldbirke>
In-Reply-To: <20120927064338.GA4789@sigill.intra.peff.net>

On Thu, Sep 27, 2012 at 02:43:38AM -0400, Jeff King wrote:

> Here are the numbers using sed[1]
> instead:

Show 20 quoted lines

> -# Quotes each element of an IFS-delimited list for shell reuse
> +# Quotes each element of a newline-delimited list for shell reuse
>  __git_quote()
>  {
> -	local i
> -	local delim
> -	for i in $1; do
> -		local quoted=${i//\'/\'\\\'\'}
> -		printf "${delim:+$IFS}'%s'" "$quoted"
> -		delim=t
> -	done
> +	echo "$1" |
> +	sed "
> +	  s/'/'\\\\''/g
> +	  s/^/'/
> +	  s/$/'/
> +	"
>  }
>  
>  # Generates completion reply with compgen, appending a space to possible

Your usage of sed got me thinking and come up with this.

The first two fix benign bugs in completion tests, and the third adds tests for __gitcomp_nl(). These are good to go.

The fourth adds __gitcomp() and __gitcomp_nl() tests for this expansion breakage. The expected results might need some adjustments, because they contain special characters unquoted, but I'm tempted to say that a branch called $foo is so rare in practice, that we shouldn't bother.

The final one is a proof of concept for inspiration. It gets rid of compgen and its crazy additional expansion replacing it with a small sed script. It needs further work to handle words with whitespaces and special characters.

SZEDER Gábor (5):
  completion: fix non-critical bugs in __gitcomp() tests
  completion: fix args of run_completion() test helper
  completion: add tests for the __gitcomp_nl() completion helper
    function
  completion: test __gitcomp() and __gitcomp_nl() with expandable words
  completion: avoid compgen to fix expansion issues in __gitcomp_nl()

 contrib/completion/git-completion.bash |  6 ++-
 t/t9902-completion.sh                  | 91 +++++++++++++++++++++++++++++++---
 2 files changed, 90 insertions(+), 7 deletions(-)