From: Brian Gernhardt Date: Wed, 27 Oct 2010 17:23:27 GMT Subject: Re: [PATCH v2/RFC] Make git-completion Bash 4 compatible. Message-ID: <091B05F5-D5F4-4DD6-90AF-E0BD77700326@gernhardtsoftware.com> In-Reply-To: <20101027131506.4da06c6d@MonteCarlo> On Oct 27, 2010, at 1:15 PM, Peter van der Does wrote: > contrib/completion/git-completion.bash | 417 +++++++++++++++++++++++++++----- > 1 files changed, 355 insertions(+), 62 deletions(-) That's a lot of added lines? Do we use all of this code? If not, should we trim it down just to support the features we need? Also, there appears to be no note in the code or commit message that this came from somewhere else. Shouldn't we note the source of the code? Is bash-completion GPLv2 as we are? > + _upvar() { > + if unset -v "$1"; then # Unset & validate varname Nit: This should be indented one less level. > + if (( $# == 2 )); then > + eval $1=\"\$2\" # Return single value > + else > + eval $1=\(\"\${@:2}\"\) # Return array > + fi > + fi > + } > + Other than those concerns, I like it. Good call not trying to redefine the functions from bash-completion if it's loaded already. ~~ Brian