Subject: Re: [RFC/PATCH 2/1] bash: eliminate dependency on bash_completion lib

From: SZEDER Gábor <[email-masked]>
Date: Fri, 3 Dec 2010 00:40:53 +0100

Hi,

On Thu, Dec 02, 2010 at 03:02:07PM -0600, Jonathan Nieder wrote:
> Add a minimal implementation of _get_comp_words_by_ref,
> the routine used to work around bash 4.0's COMP_WORDS semantics.

Some extension:

Bash's programmable completion provides the COMP_WORDS array variable,
which holds the individual words in the current command line.  In Bash
versions prior to v4 "words are split on shell metacharacters as the
shell parser would separate them" (quote from bash v3.2.48's man
page).  This behavior has changed with Bash v4, and the command line
"is split into words as readline would split it, using COMP_WORDBREAKS
as" "the set of characters that the readline library treats as word
separators" (quote from bash v4's man page).

Since COMP_WORDBREAKS contains the characters : and = by default, this
behavior change in Bash also affects git's completion script.  For
example, when using Bash v4 the completion script can't provide
possible options for a command line argument (e.g. git log
--pretty=<TAB><TAB> lists files, but it should list possible log
formats).

I would really, _really_ like to have the above text in the commit
message (either in yours or in Peter's), because it took me weeks to
figure this out ;)  Not that it was that difficult, but when I
discovered this issue more than a month ago, "unfortunately" I
remembered a similar issue (db8a9ff, bash completion: Resolve git show
ref:path<tab> losing ref: portion, 2008-07-15), and it got me
sidetracked really really  badly.

(I'm still wondering what Bash v3.x was doing with COMP_WORDBREAKS,
though...)

> Based on bash-completion 2.x (commit bf763033, 2010-10-26) but
> tweaked for simplicity and to allow zsh to at least parse the
> code.
> 
> Based-on-patch-by: Peter van der Does <[email-masked]>
> Signed-off-by: Jonathan Nieder <[email-masked]>
> ---
> Peter van der Does wrote:
> > Jonathan Nieder <[email-masked]> wrote:
> 
> >>  2. Import the definition of _get_comp_words_by_ref from the
> >>     bash-completion lib and use it if ZSH_VERSION is unset.
> >> 
> >>  3. Further refinements, if needed.
> >> 
> >> What do you think?
> >
> > I like the idea and we should go with this solution.
> > 
> > If by importing you mean using :
> > [CODE]. /git_bash_completion-functions[/CODE] in the
> > contrib/completion/git-completion.bash script, which would be the best
> > solution imho. The question is where to place that the function file.
> [...]
> > It would have to include copying the functions file somewhere as well.
> > 
> > Or we could use the method used now and include the functions in the
> > git-completion.bash script.
> 
> Sorry for the lack of clarity.  Here's what I meant.
> 
>  contrib/completion/git-completion.bash |  125 ++++++++++++++++++++++++++++++++
>  1 files changed, 125 insertions(+), 0 deletions(-)

I haven't tried to understand the code yet, but noticed the following
two nits while glancing it over.

> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 0b0eb45..1743319 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -327,7 +327,102 @@ __gitcomp_1 ()
>  	done
>  }
>  
> +# The following function is based on code from:
> +#
> +#   bash_completion - programmable completion functions for bash 3.2+
> +#
> +#   Copyright © 2006-2008, Ian Macdonald <[email-masked]>
> +#             © 2009-2010, Bash Completion Maintainers
> +#                     <[email-masked]>
> +#
> +#   This program is free software; you can redistribute it and/or modify
> +#   it under the terms of the GNU General Public License as published by
> +#   the Free Software Foundation; either version 2, or (at your option)
> +#   any later version.
> +#
> +#   This program is distributed in the hope that it will be useful,
> +#   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +#   GNU General Public License for more details.
> +#
> +#   You should have received a copy of the GNU General Public License
> +#   along with this program; if not, write to the Free Software Foundation,
> +#   Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> +#
> +#   The latest version of this software can be obtained here:
> +#
> +#   http://bash-completion.alioth.debian.org/
> +#
> +#   RELEASE: 2.x
> +
> +# This function can be used to access a tokenized list of words
> +# on the command line:
> +#
> +#	__reassemble_comp_words_by_ref '=:'

__git_reassemble_comp_words_by_ref?

> +#	if test "${words_[cword_-1]}" = -w
> +#	then
> +#		...
> +#	fi
> +#
> +# The argument should be a collection of characters from the list of
> +# word completion separators (COMP_WORDBREAKS) to treat as ordinary
> +# characters.
> +#
> +# This is roughly equivalent to locally setting COMP_WORDBREAKS to
> +# exclude those characters, but it does not clobber COMP_WORDBREAKS.
> +# The intent is for it to be used by commands like ssh that want to
> +# treat host:path as one token.
> +#
> +# Output: words_, cword_, cur_.
> +
> +__git_reassemble_comp_words_by_ref()
> +{
> +	local exclude i j first
> +	# Which word separators to exclude?
> +	exclude="${1//[^$COMP_WORDBREAKS]}"
> +	cword_=$COMP_CWORD
> +	if [ -z "$exclude" ]; then
> +		words_=("${COMP_WORDS[@]}")
> +		return
> +	fi
> +	# List of word completion separators has shrunk;
> +	# re-assemble words to complete.
> +	for ((i=0, j=0; i < ${#COMP_WORDS[@]}; i++, j++)); do
> +		# Append each nonempty word consisting of just
> +		# word separator characters to the current word.
> +		first=t
> +		while
> +			[ $i -gt 0 ] &&
> +			[ -n "${COMP_WORDS[$i]}" ] &&
> +			# word consists of excluded word separators
> +			[ "${COMP_WORDS[$i]//[^$exclude]}" = "${COMP_WORDS[$i]}" ]
> +		do
> +			# Attach to the previous token,
> +			# unless the previous token is the command name.
> +			if [ $j -ge 2 ] && [ -n "$first" ]; then
> +				((j--))
> +			fi
> +			first=
> +			words_[$j]=${words_[j]}${COMP_WORDS[i]}
> +			if [ $i = $COMP_CWORD ]; then
> +				cword_=$j
> +			fi
> +			if (($i < ${#COMP_WORDS[@]} - 1)); then
> +				((i++))
> +			else
> +				# Done.
> +				return
> +			fi
> +		done
> +		words_[$j]=${words_[j]}${COMP_WORDS[i]}
> +		if [ $i = $COMP_CWORD ]; then
> +			cword_=$j
> +		fi
> +	done
> +}
> +
>  if ! type _get_comp_words_by_ref >/dev/null 2>&1; then
> +if [[ -n $ZSH_VERSION ]]; then

This should be ${ZSH_VERSION-} to keep 'set -u' environments happy.