All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2/RFC] Make git-completion Bash 4 compatible.
@ 2010-10-27 17:15 Peter van der Does
  2010-10-27 17:23 ` Brian Gernhardt
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Peter van der Does @ 2010-10-27 17:15 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: git, SZEDER Gábor, Jonathan Nieder, Marc Branchaud,
	Brian Gernhardt, Kevin Ballard, Mathias Lafeldt


The completion script does not work as expected under Bash 4.
Bash: 3
output:
$ git log --pretty=<tab><tab>
email     full      medium    raw
format:   fuller    oneline   short

Bash: 4
output:
$ git log --pretty=<tab><tab>
.bash_logout         .local/
.bash_profile        Music/
--More--

Signed-off-by: Peter van der Does <peter@avirtualhome.com>
---
Updated patch to play along with latest maint branch. It broke after the patch 
ml/completion-zsh was implemented.
Removed some debugging code from the patch.

 contrib/completion/git-completion.bash |  417 +++++++++++++++++++++++++++-----
 1 files changed, 355 insertions(+), 62 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 64341d5..85fb0f1 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -76,12 +76,251 @@
 #
 #       git@vger.kernel.org
 #
+# Updated for Bash 4.0
 
 case "$COMP_WORDBREAKS" in
 *:*) : great ;;
 *)   COMP_WORDBREAKS="$COMP_WORDBREAKS:"
 esac
 
+# If the function _get_comp_words_by_ref does not exists, we can assume the
+# bash_completion 1.2 script isn't loaded and therefor we're defining the
+# necessary functions ourselves.
+if ! type _get_comp_words_by_ref &> /dev/null ; then
+	# Assign variable one scope above the caller
+	# Usage: local "$1" && _upvar $1 "value(s)"
+	# Param: $1  Variable name to assign value to
+	# Param: $*  Value(s) to assign.  If multiple values, an array is
+	#            assigned, otherwise a single value is assigned.
+	# NOTE: For assigning multiple variables, use '_upvars'.  Do NOT
+	#       use multiple '_upvar' calls, since one '_upvar' call might
+	#       reassign a variable to be used by another '_upvar' call.
+	# See: http://fvue.nl/wiki/Bash:_Passing_variables_by_reference
+	_upvar() {
+		if unset -v "$1"; then           # Unset & validate varname
+	        if (( $# == 2 )); then
+	            eval $1=\"\$2\"          # Return single value
+	        else
+	            eval $1=\(\"\${@:2}\"\)  # Return array
+	        fi
+	    fi
+	}
+
+
+	# Assign variables one scope above the caller
+	# Usage: local varname [varname ...] &&
+	#        _upvars [-v varname value] | [-aN varname [value ...]] ...
+	# Available OPTIONS:
+	#     -aN  Assign next N values to varname as array
+	#     -v   Assign single value to varname
+	# Return: 1 if error occurs
+	# See: http://fvue.nl/wiki/Bash:_Passing_variables_by_reference
+	_upvars() {
+	    if ! (( $# )); then
+	        echo "${FUNCNAME[0]}: usage: ${FUNCNAME[0]} [-v varname"\
+	            "value] | [-aN varname [value ...]] ..." 1>&2
+	        return 2
+	    fi
+	    while (( $# )); do
+	        case $1 in
+	            -a*)
+	                # Error checking
+	                [[ ${1#-a} ]] || { echo "bash: ${FUNCNAME[0]}: \`$1': missing"\
+	                    "number specifier" 1>&2; return 1; }
+	                printf %d "${1#-a}" &> /dev/null || { echo "bash:"\
+	                    "${FUNCNAME[0]}: \`$1': invalid number specifier" 1>&2
+	                    return 1; }
+	                # Assign array of -aN elements
+	                [[ "$2" ]] && unset -v "$2" && eval $2=\(\"\${@:3:${1#-a}}\"\) &&
+	                shift $((${1#-a} + 2)) || { echo "bash: ${FUNCNAME[0]}:"\
+	                    "\`$1${2+ }$2': missing argument(s)" 1>&2; return 1; }
+	                ;;
+	            -v)
+	                # Assign single value
+	                [[ "$2" ]] && unset -v "$2" && eval $2=\"\$3\" &&
+	                shift 3 || { echo "bash: ${FUNCNAME[0]}: $1: missing"\
+	                "argument(s)" 1>&2; return 1; }
+	                ;;
+	            *)
+	                echo "bash: ${FUNCNAME[0]}: $1: invalid option" 1>&2
+	                return 1 ;;
+	        esac
+	    done
+	}
+
+
+	# Reassemble command line words, excluding specified characters from the
+	# list of word completion separators (COMP_WORDBREAKS).
+	# @param $1 chars  Characters out of $COMP_WORDBREAKS which should
+	#     NOT be considered word breaks. This is useful for things like scp where
+	#     we want to return host:path and not only path, so we would pass the
+	#     colon (:) as $1 here.
+	# @param $2 words  Name of variable to return words to
+	# @param $3 cword  Name of variable to return cword to
+	#
+	__reassemble_comp_words_by_ref() {
+	    local exclude i j ref
+	    # Exclude word separator characters?
+	    if [[ $1 ]]; then
+	        # Yes, exclude word separator characters;
+	        # Exclude only those characters, which were really included
+	        exclude="${1//[^$COMP_WORDBREAKS]}"
+	    fi
+
+	    # Default to cword unchanged
+	    eval $3=$COMP_CWORD
+	    # Are characters excluded which were former included?
+	    if [[ $exclude ]]; then
+	        # Yes, list of word completion separators has shrunk;
+	        # Re-assemble words to complete
+	        for (( i=0, j=0; i < ${#COMP_WORDS[@]}; i++, j++)); do
+	            # Is current word not word 0 (the command itself) and is word not
+	            # empty and is word made up of just word separator characters to be
+	            # excluded?
+	            while [[ $i -gt 0 && ${COMP_WORDS[$i]} &&
+	                ${COMP_WORDS[$i]//[^$exclude]} == ${COMP_WORDS[$i]}
+	            ]]; do
+	                [ $j -ge 2 ] && ((j--))
+	                # Append word separator to current word
+	                ref="$2[$j]"
+	                eval $2[$j]=\${!ref}\${COMP_WORDS[i]}
+	                # Indicate new cword
+	                [ $i = $COMP_CWORD ] && eval $3=$j
+	                # Indicate next word if available, else end *both* while and for loop
+	                (( $i < ${#COMP_WORDS[@]} - 1)) && ((i++)) || break 2
+	            done
+	            # Append word to current word
+	            ref="$2[$j]"
+	            eval $2[$j]=\${!ref}\${COMP_WORDS[i]}
+	            # Indicate new cword
+	            [ $i = $COMP_CWORD ] && [[ ${COMP_WORDS[i]} ]] && eval $3=$j
+	        done
+	    else
+	        # No, list of word completions separators hasn't changed;
+	        eval $2=\( \"\${COMP_WORDS[@]}\" \)
+	    fi
+	} # __reassemble_comp_words_by_ref()
+
+	# @param $1 exclude  Characters out of $COMP_WORDBREAKS which should NOT be
+	#     considered word breaks. This is useful for things like scp where
+	#     we want to return host:path and not only path, so we would pass the
+	#     colon (:) as $1 in this case.  Bash-3 doesn't do word splitting, so this
+	#     ensures we get the same word on both bash-3 and bash-4.
+	# @param $2 words  Name of variable to return words to
+	# @param $3 cword  Name of variable to return cword to
+	# @param $4 cur  Name of variable to return current word to complete to
+	# @see ___get_cword_at_cursor_by_ref()
+	__get_cword_at_cursor_by_ref() {
+	    local cword words=()
+	    __reassemble_comp_words_by_ref "$1" words cword
+
+	    local i cur2
+	    local cur="$COMP_LINE"
+	    local index="$COMP_POINT"
+	    for (( i = 0; i <= cword; ++i )); do
+	        while [[
+	            # Current word fits in $cur?
+	            "${#cur}" -ge ${#words[i]} &&
+	            # $cur doesn't match cword?
+	            "${cur:0:${#words[i]}}" != "${words[i]}"
+	        ]]; do
+	            # Strip first character
+	            cur="${cur:1}"
+	            # Decrease cursor position
+	            ((index--))
+	        done
+
+	        # Does found word matches cword?
+	        if [[ "$i" -lt "$cword" ]]; then
+	            # No, cword lies further;
+	            local old_size="${#cur}"
+	            cur="${cur#${words[i]}}"
+	            local new_size="${#cur}"
+	            index=$(( index - old_size + new_size ))
+	        fi
+	    done
+
+	    if [[ "${words[cword]:0:${#cur}}" != "$cur" ]]; then
+	        # We messed up. At least return the whole word so things keep working
+	        cur2=${words[cword]}
+	    else
+	        cur2=${cur:0:$index}
+	    fi
+
+	    local "$2" "$3" "$4" &&
+	        _upvars -a${#words[@]} $2 "${words[@]}" -v $3 "$cword" -v $4 "$cur2"
+	}
+
+
+	# Get the word to complete and optional previous words.
+	# This is nicer than ${COMP_WORDS[$COMP_CWORD]}, since it handles cases
+	# where the user is completing in the middle of a word.
+	# (For example, if the line is "ls foobar",
+	# and the cursor is here -------->   ^
+	# Also one is able to cross over possible wordbreak characters.
+	# Usage: _get_comp_words_by_ref [OPTIONS] [VARNAMES]
+	# Available VARNAMES:
+	#     cur         Return cur via $cur
+	#     prev        Return prev via $prev
+	#     words       Return words via $words
+	#     cword       Return cword via $cword
+	#
+	# Available OPTIONS:
+	#     -n EXCLUDE  Characters out of $COMP_WORDBREAKS which should NOT be
+	#                 considered word breaks. This is useful for things like scp
+	#                 where we want to return host:path and not only path, so we
+	#                 would pass the colon (:) as -n option in this case.  Bash-3
+	#                 doesn't do word splitting, so this ensures we get the same
+	#                 word on both bash-3 and bash-4.
+	#     -c VARNAME  Return cur via $VARNAME
+	#     -p VARNAME  Return prev via $VARNAME
+	#     -w VARNAME  Return words via $VARNAME
+	#     -i VARNAME  Return cword via $VARNAME
+	#
+	# Example usage:
+	#
+	#    $ _get_comp_words_by_ref -n : cur prev
+	#
+	_get_comp_words_by_ref()
+	{
+	    local exclude flag i OPTIND=1
+	    local cur cword words=()
+	    local upargs=() upvars=() vcur vcword vprev vwords
+
+	    while getopts "c:i:n:p:w:" flag "$@"; do
+	        case $flag in
+	            c) vcur=$OPTARG ;;
+	            i) vcword=$OPTARG ;;
+	            n) exclude=$OPTARG ;;
+	            p) vprev=$OPTARG ;;
+	            w) vwords=$OPTARG ;;
+	        esac
+	    done
+	    while [[ $# -ge $OPTIND ]]; do
+	        case ${!OPTIND} in
+	            cur)   vcur=cur ;;
+	            prev)  vprev=prev ;;
+	            cword) vcword=cword ;;
+	            words) vwords=words ;;
+	            *) echo "bash: $FUNCNAME(): \`${!OPTIND}': unknown argument" \
+	                1>&2; return 1
+	        esac
+	        let "OPTIND += 1"
+	    done
+
+	    __get_cword_at_cursor_by_ref "$exclude" words cword cur
+
+	    [[ $vcur   ]] && { upvars+=("$vcur"  ); upargs+=(-v $vcur   "$cur"  ); }
+	    [[ $vcword ]] && { upvars+=("$vcword"); upargs+=(-v $vcword "$cword"); }
+	    [[ $vprev  ]] && { upvars+=("$vprev" ); upargs+=(-v $vprev
+	        "${words[cword - 1]}"); }
+	    [[ $vwords ]] && { upvars+=("$vwords"); upargs+=(-a${#words[@]} $vwords
+	        "${words[@]}"); }
+
+	    (( ${#upvars[@]} )) && local "${upvars[@]}" && _upvars "${upargs[@]}"
+	}
+fi
+
 # __gitdir accepts 0 or 1 arguments (i.e., location)
 # returns location of .git repo
 __gitdir ()
@@ -331,7 +570,8 @@ __gitcomp_1 ()
 # generates completion reply with compgen
 __gitcomp ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref -n "=" cur
 	if [ $# -gt 2 ]; then
 		cur="$3"
 	fi
@@ -390,7 +630,8 @@ __git_tags ()
 __git_refs ()
 {
 	local i is_hash=y dir="$(__gitdir "${1-}")"
-	local cur="${COMP_WORDS[COMP_CWORD]}" format refs
+	local cur format refs
+	_get_comp_words_by_ref cur
 	if [ -d "$dir" ]; then
 		case "$cur" in
 		refs|refs/*)
@@ -488,7 +729,8 @@ __git_compute_merge_strategies ()
 
 __git_complete_file ()
 {
-	local pfx ls ref cur="${COMP_WORDS[COMP_CWORD]}"
+	local pfx ls ref cur
+	_get_comp_words_by_ref -n ":" cur
 	case "$cur" in
 	?*:*)
 		ref="${cur%%:*}"
@@ -536,7 +778,8 @@ __git_complete_file ()
 
 __git_complete_revlist ()
 {
-	local pfx cur="${COMP_WORDS[COMP_CWORD]}"
+	local pfx cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	*...*)
 		pfx="${cur%...*}..."
@@ -556,11 +799,14 @@ __git_complete_revlist ()
 
 __git_complete_remote_or_refspec ()
 {
-	local cmd="${COMP_WORDS[1]}"
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local git_comp_words git_comp_cword
+	__reassemble_comp_words_by_ref : git_comp_words git_comp_cword
+	local cmd="${git_comp_words[1]}"
+	local cur
 	local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
-	while [ $c -lt $COMP_CWORD ]; do
-		i="${COMP_WORDS[c]}"
+	_get_comp_words_by_ref -n ":" cur
+	while [ $c -lt $git_comp_cword ]; do
+		i="${git_comp_words[c]}"
 		case "$i" in
 		--mirror) [ "$cmd" = "push" ] && no_complete_refspec=1 ;;
 		--all)
@@ -628,13 +874,15 @@ __git_complete_remote_or_refspec ()
 
 __git_complete_strategy ()
 {
+	local cur prev
+	_get_comp_words_by_ref -n "=" cur prev
 	__git_compute_merge_strategies
-	case "${COMP_WORDS[COMP_CWORD-1]}" in
+	case "${prev}" in
 	-s|--strategy)
 		__gitcomp "$__git_merge_strategies"
 		return 0
 	esac
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+
 	case "$cur" in
 	--strategy=*)
 		__gitcomp "$__git_merge_strategies" "" "${cur##--strategy=}"
@@ -824,7 +1072,10 @@ __git_whitespacelist="nowarn warn error error-all fix"
 
 _git_am ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}" dir="$(__gitdir)"
+	local cur
+	local dir="$(__gitdir)"
+
+	_get_comp_words_by_ref -n "=" cur
 	if [ -d "$dir"/rebase-apply ]; then
 		__gitcomp "--skip --continue --resolved --abort"
 		return
@@ -848,7 +1099,8 @@ _git_am ()
 
 _git_apply ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref -n "=" cur
 	case "$cur" in
 	--whitespace=*)
 		__gitcomp "$__git_whitespacelist" "" "${cur##--whitespace=}"
@@ -871,7 +1123,8 @@ _git_add ()
 {
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -885,7 +1138,8 @@ _git_add ()
 
 _git_archive ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref -n "=" cur
 	case "$cur" in
 	--format=*)
 		__gitcomp "$(git archive --list)" "" "${cur##--format=}"
@@ -929,7 +1183,8 @@ _git_bisect ()
 
 _git_branch ()
 {
-	local i c=1 only_local_ref="n" has_r="n"
+	local i c=1 only_local_ref="n" has_r="n", cur
+	_get_comp_words_by_ref cur
 
 	while [ $c -lt $COMP_CWORD ]; do
 		i="${COMP_WORDS[c]}"
@@ -940,7 +1195,7 @@ _git_branch ()
 		c=$((++c))
 	done
 
-	case "${COMP_WORDS[COMP_CWORD]}" in
+	case "$cur" in
 	--*)
 		__gitcomp "
 			--color --no-color --verbose --abbrev= --no-abbrev
@@ -982,7 +1237,8 @@ _git_checkout ()
 {
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref -n "=" cur
 	case "$cur" in
 	--conflict=*)
 		__gitcomp "diff3 merge" "" "${cur##--conflict=}"
@@ -1006,7 +1262,8 @@ _git_cherry ()
 
 _git_cherry_pick ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "--edit --no-commit"
@@ -1021,7 +1278,8 @@ _git_clean ()
 {
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "--dry-run --quiet"
@@ -1033,7 +1291,8 @@ _git_clean ()
 
 _git_clone ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -1060,7 +1319,8 @@ _git_commit ()
 {
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref -n "=" cur
 	case "$cur" in
 	--cleanup=*)
 		__gitcomp "default strip verbatim whitespace
@@ -1095,7 +1355,8 @@ _git_commit ()
 
 _git_describe ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -1127,7 +1388,8 @@ _git_diff ()
 {
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "--cached --staged --pickaxe-all --pickaxe-regex
@@ -1148,7 +1410,8 @@ _git_difftool ()
 {
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref -n "=" cur
 	case "$cur" in
 	--tool=*)
 		__gitcomp "$__git_mergetools_common kompare" "" "${cur##--tool=}"
@@ -1173,7 +1436,8 @@ __git_fetch_options="
 
 _git_fetch ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "$__git_fetch_options"
@@ -1185,7 +1449,8 @@ _git_fetch ()
 
 _git_format_patch ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref -n "=" cur
 	case "$cur" in
 	--thread=*)
 		__gitcomp "
@@ -1217,7 +1482,8 @@ _git_format_patch ()
 
 _git_fsck ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -1232,7 +1498,8 @@ _git_fsck ()
 
 _git_gc ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "--prune --aggressive"
@@ -1251,7 +1518,8 @@ _git_grep ()
 {
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -1274,7 +1542,8 @@ _git_grep ()
 
 _git_help ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "--all --info --man --web"
@@ -1292,7 +1561,8 @@ _git_help ()
 
 _git_init ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref -n "=" cur
 	case "$cur" in
 	--shared=*)
 		__gitcomp "
@@ -1312,7 +1582,8 @@ _git_ls_files ()
 {
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "--cached --deleted --modified --others --ignored
@@ -1366,7 +1637,8 @@ _git_log ()
 {
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref -n "=" cur
 	local g="$(git rev-parse --git-dir 2>/dev/null)"
 	local merge=""
 	if [ -f "$g/MERGE_HEAD" ]; then
@@ -1425,7 +1697,8 @@ _git_merge ()
 {
 	__git_complete_strategy && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "$__git_merge_options"
@@ -1436,7 +1709,8 @@ _git_merge ()
 
 _git_mergetool ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref -n "=" cur
 	case "$cur" in
 	--tool=*)
 		__gitcomp "$__git_mergetools_common tortoisemerge" "" "${cur##--tool=}"
@@ -1457,7 +1731,8 @@ _git_merge_base ()
 
 _git_mv ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "--dry-run"
@@ -1494,7 +1769,8 @@ _git_pull ()
 {
 	__git_complete_strategy && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -1510,8 +1786,9 @@ _git_pull ()
 
 _git_push ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
-	case "${COMP_WORDS[COMP_CWORD-1]}" in
+	local cur prev
+	_get_comp_words_by_ref -n "=" cur prev
+	case "$prev" in
 	--repo)
 		__gitcomp "$(__git_remotes)"
 		return
@@ -1534,7 +1811,9 @@ _git_push ()
 
 _git_rebase ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}" dir="$(__gitdir)"
+	local cur
+	local dir="$(__gitdir)"
+	_get_comp_words_by_ref -n "=" cur
 	if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
 		__gitcomp "--continue --skip --abort"
 		return
@@ -1564,7 +1843,8 @@ __git_send_email_suppresscc_options="author self cc bodycc sob cccmd body all"
 
 _git_send_email ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref -n "=" cur
 	case "$cur" in
 	--confirm=*)
 		__gitcomp "
@@ -1606,9 +1886,11 @@ _git_stage ()
 
 __git_config_get_set_variables ()
 {
-	local prevword word config_file= c=$COMP_CWORD
+	local prevword word config_file= c
+	local git_comp_words
+	__reassemble_comp_words_by_ref = git_comp_words c
 	while [ $c -gt 1 ]; do
-		word="${COMP_WORDS[c]}"
+		word="${git_comp_words[c]}"
 		case "$word" in
 		--global|--system|--file=*)
 			config_file="$word"
@@ -1636,9 +1918,9 @@ __git_config_get_set_variables ()
 
 _git_config ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
-	local prv="${COMP_WORDS[COMP_CWORD-1]}"
-	case "$prv" in
+	local cur prev
+	_get_comp_words_by_ref cur prev
+	case "$prev" in
 	branch.*.remote)
 		__gitcomp "$(__git_remotes)"
 		return
@@ -1648,13 +1930,13 @@ _git_config ()
 		return
 		;;
 	remote.*.fetch)
-		local remote="${prv#remote.}"
+		local remote="${prev#remote.}"
 		remote="${remote%.fetch}"
 		__gitcomp "$(__git_refs_remotes "$remote")"
 		return
 		;;
 	remote.*.push)
-		local remote="${prv#remote.}"
+		local remote="${prev#remote.}"
 		remote="${remote%.push}"
 		__gitcomp "$(git --git-dir="$(__gitdir)" \
 			for-each-ref --format='%(refname):%(refname)' \
@@ -2045,7 +2327,8 @@ _git_reset ()
 {
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "--merge --mixed --hard --soft --patch"
@@ -2057,7 +2340,8 @@ _git_reset ()
 
 _git_revert ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "--edit --mainline --no-edit --no-commit --signoff"
@@ -2071,7 +2355,8 @@ _git_rm ()
 {
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur=
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "--cached --dry-run --ignore-unmatch --quiet"
@@ -2085,7 +2370,8 @@ _git_shortlog ()
 {
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -2103,7 +2389,8 @@ _git_show ()
 {
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref -n "=" cur
 	case "$cur" in
 	--pretty=*)
 		__gitcomp "$__git_log_pretty_formats
@@ -2127,7 +2414,8 @@ _git_show ()
 
 _git_show_branch ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -2144,10 +2432,11 @@ _git_show_branch ()
 
 _git_stash ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
 	local save_opts='--keep-index --no-keep-index --quiet --patch'
 	local subcommands='save list show apply clear drop pop create branch'
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+	_get_comp_words_by_ref cur
 	if [ -z "$subcommand" ]; then
 		case "$cur" in
 		--*)
@@ -2189,7 +2478,8 @@ _git_submodule ()
 
 	local subcommands="add status init update summary foreach sync"
 	if [ -z "$(__git_find_on_cmdline "$subcommands")" ]; then
-		local cur="${COMP_WORDS[COMP_CWORD]}"
+		local cur
+		_get_comp_words_by_ref cur
 		case "$cur" in
 		--*)
 			__gitcomp "--quiet --cached"
@@ -2233,7 +2523,8 @@ _git_svn ()
 			--edit --rmdir --find-copies-harder --copy-similarity=
 			"
 
-		local cur="${COMP_WORDS[COMP_CWORD]}"
+		local cur
+		_get_comp_words_by_ref cur
 		case "$subcommand,$cur" in
 		fetch,--*)
 			__gitcomp "--revision= --fetch-all $fc_opts"
@@ -2343,15 +2634,16 @@ _git_whatchanged ()
 
 _git ()
 {
-	local i c=1 command __git_dir
+	local i c=1 command __git_dir git_comp_words git_comp_cword
 
 	if [[ -n $ZSH_VERSION ]]; then
 		emulate -L bash
 		setopt KSH_TYPESET
 	fi
 
-	while [ $c -lt $COMP_CWORD ]; do
-		i="${COMP_WORDS[c]}"
+	__reassemble_comp_words_by_ref = git_comp_words git_comp_cword
+	while [ $c -lt $git_comp_cword ]; do
+		i="${git_comp_words[c]}"
 		case "$i" in
 		--git-dir=*) __git_dir="${i#--git-dir=}" ;;
 		--bare)      __git_dir="." ;;
@@ -2363,7 +2655,7 @@ _git ()
 	done
 
 	if [ -z "$command" ]; then
-		case "${COMP_WORDS[COMP_CWORD]}" in
+		case "${git_comp_words[git_comp_cword]}" in
 		--*)   __gitcomp "
 			--paginate
 			--no-pager
@@ -2401,12 +2693,13 @@ _gitk ()
 
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
 	local g="$(__gitdir)"
 	local merge=""
 	if [ -f "$g/MERGE_HEAD" ]; then
 		merge="--merge"
 	fi
+	_get_comp_words_by_ref cur
 	case "$cur" in
 	--*)
 		__gitcomp "
-- 
1.7.3.2


-- 
Peter van der Does

GPG key: E77E8E98

IRC: Ganseki on irc.freenode.net
Twitter: @petervanderdoes

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2/RFC] Make git-completion Bash 4 compatible.
  2010-10-27 17:15 [PATCH v2/RFC] Make git-completion Bash 4 compatible Peter van der Does
@ 2010-10-27 17:23 ` Brian Gernhardt
  2010-10-27 17:31 ` Jonathan Nieder
  2010-10-27 22:39 ` SZEDER Gábor
  2 siblings, 0 replies; 11+ messages in thread
From: Brian Gernhardt @ 2010-10-27 17:23 UTC (permalink / raw)
  To: Peter van der Does
  Cc: Shawn O. Pearce, git, SZEDER Gábor, Jonathan Nieder,
	Marc Branchaud, Kevin Ballard, Mathias Lafeldt


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2/RFC] Make git-completion Bash 4 compatible.
  2010-10-27 17:15 [PATCH v2/RFC] Make git-completion Bash 4 compatible Peter van der Does
  2010-10-27 17:23 ` Brian Gernhardt
@ 2010-10-27 17:31 ` Jonathan Nieder
  2010-10-27 22:53   ` SZEDER Gábor
  2010-10-27 22:39 ` SZEDER Gábor
  2 siblings, 1 reply; 11+ messages in thread
From: Jonathan Nieder @ 2010-10-27 17:31 UTC (permalink / raw)
  To: Peter van der Does
  Cc: Shawn O. Pearce, git, SZEDER Gábor, Marc Branchaud,
	Brian Gernhardt, Kevin Ballard, Mathias Lafeldt

Hi Peter,

Peter van der Does wrote:

> The completion script does not work as expected under Bash 4.

Thanks for your work fixing this.  That's awesome.

It would be ideal if someone could write or find a nice summary of the
problem and the chosen solution, for inclusion in the commit message.

Could some zsh user perhaps test that the new zsh support is not
broken?

>  1 files changed, 355 insertions(+), 62 deletions(-)

Kind of unfortunate.  There are a lot of comments, but still...

> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -76,12 +76,251 @@
>  #
>  #       git@vger.kernel.org
>  #
> +# Updated for Bash 4.0

I don't think this comment will be so important for posterity (e.g., once
bash 5 comes around ;-)).

[...]
> +# If the function _get_comp_words_by_ref does not exists, we can assume the
> +# bash_completion 1.2 script isn't loaded and therefor we're defining the
> +# necessary functions ourselves.

Probably this explanation belongs in the commit message?  A comment
could provide a brief reminder, like:

	if ! type _get_comp_words_by_ref &>/dev/null ; then
		# The bash_completion 1.2 library was not loaded,
		# so we have to define some functions from it ourselves.

Are the implementations taken from bash_completion?  If so, that would
be very useful information for the log message: future readers may
want to know where to look for a more recent version.

> +	# Assign variable one scope above the caller
[... I'm assuming this is all written correctly, etc ...]

> @@ -331,7 +570,8 @@ __gitcomp_1 ()
>  # generates completion reply with compgen
>  __gitcomp ()
>  {
> -	local cur="${COMP_WORDS[COMP_CWORD]}"
> +	local cur
> +	_get_comp_words_by_ref -n "=" cur
[...]

The rest looks sane.  Maybe it would make sense to split this into two
patches for readability:

 - one to introduce the _get_comp_words_by_ref function
 - one to use it

?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2/RFC] Make git-completion Bash 4 compatible.
  2010-10-27 17:15 [PATCH v2/RFC] Make git-completion Bash 4 compatible Peter van der Does
  2010-10-27 17:23 ` Brian Gernhardt
  2010-10-27 17:31 ` Jonathan Nieder
@ 2010-10-27 22:39 ` SZEDER Gábor
  2010-10-28  0:48   ` Jonathan Nieder
  2 siblings, 1 reply; 11+ messages in thread
From: SZEDER Gábor @ 2010-10-27 22:39 UTC (permalink / raw)
  To: Peter van der Does
  Cc: Shawn O. Pearce, git, Jonathan Nieder, Marc Branchaud,
	Brian Gernhardt, Kevin Ballard, Mathias Lafeldt

Hi,

On Wed, Oct 27, 2010 at 01:15:06PM -0400, Peter van der Does wrote:
> The completion script does not work as expected under Bash 4.

Thank you.  I applied your patch to play around a bit, and it seems to
work (but, of course, I have not tried all the affected commands).

I agree with the comments and suggestiong of Brian and Jonathan.  The
current commit message covers only the symptoms of a bug this patch
attempts to fix, but it should also explain its cause and how the
patch is supposed to fix it.  Unfortunately, I can only comment on the
latter: if these new functions are good enough for the folks over at
bash-completion project, then they should be good enough for us (;

I'm still puzzled that the only relevant entry I could find in the
bash NEWS file is:

i.  The programmable completion code now uses the same set of characters as
    readline when breaking the command line into a list of words.

Yet, as I mentioned in one of the previous threads, I have two
machines with different bash versions (3.2 and 4.1) but with the exact
same set of characters in COMP_WORDBREAKS, and they show different
behavior.

> Bash: 3
> output:
> $ git log --pretty=<tab><tab>
> email     full      medium    raw
> format:   fuller    oneline   short
> 
> Bash: 4
> output:
> $ git log --pretty=<tab><tab>
> .bash_logout         .local/
> .bash_profile        Music/
> --More--
> 
> Signed-off-by: Peter van der Does <peter@avirtualhome.com>


> @@ -556,11 +799,14 @@ __git_complete_revlist ()
>  
>  __git_complete_remote_or_refspec ()
>  {
> -	local cmd="${COMP_WORDS[1]}"
> -	local cur="${COMP_WORDS[COMP_CWORD]}"
> +	local git_comp_words git_comp_cword
> +	__reassemble_comp_words_by_ref : git_comp_words git_comp_cword

I suggest using '_get_comp_words_by_ref -n ":" words cword' here.
First, for consistency's sake, because you use
_get_comp_words_by_ref() everywhere else, too (almost).  Second, I'm
worried about the double underscore prefix in the function name,
because it usually indicates something that is not supposed to be used
directly from outside.  Even the bash-completion project's scripts
prefer _get_comp_words_by_ref() to __reassemble_comp_words_by_ref().
And third, ...

> +	local cmd="${git_comp_words[1]}"
> +	local cur
>  	local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
> -	while [ $c -lt $COMP_CWORD ]; do
> -		i="${COMP_WORDS[c]}"
> +	_get_comp_words_by_ref -n ":" cur

... you could then join this _get_comp_words_by_ref() invocation with
the one above.

> +	while [ $c -lt $git_comp_cword ]; do
> +		i="${git_comp_words[c]}"
>  		case "$i" in
>  		--mirror) [ "$cmd" = "push" ] && no_complete_refspec=1 ;;
>  		--all)


> @@ -824,7 +1072,10 @@ __git_whitespacelist="nowarn warn error error-all fix"
>  
>  _git_am ()
>  {
> -	local cur="${COMP_WORDS[COMP_CWORD]}" dir="$(__gitdir)"
> +	local cur
> +	local dir="$(__gitdir)"

You could keep the two declarations in a single line.

> +
> +	_get_comp_words_by_ref -n "=" cur
>  	if [ -d "$dir"/rebase-apply ]; then
>  		__gitcomp "--skip --continue --resolved --abort"
>  		return


> @@ -929,7 +1183,8 @@ _git_bisect ()
>  
>  _git_branch ()
>  {
> -	local i c=1 only_local_ref="n" has_r="n"
> +	local i c=1 only_local_ref="n" has_r="n", cur

The comma before "cur" shouldn't be there.

> +	_get_comp_words_by_ref cur
>  
>  	while [ $c -lt $COMP_CWORD ]; do
>  		i="${COMP_WORDS[c]}"

Hmph.

I don't think there is anything that could go wrong here with respect
to word breaking, but at least for consistency's sake you could query
for words and cword, too, and use ${words[cword]} here.


> @@ -1006,7 +1262,8 @@ _git_cherry ()
>  
>  _git_cherry_pick ()
>  {
> -	local cur="${COMP_WORDS[COMP_CWORD]}"
> +	local cur
> +_get_comp_words_by_ref cur

Missing indentation.

>  	case "$cur" in
>  	--*)
>  		__gitcomp "--edit --no-commit"


> @@ -1606,9 +1886,11 @@ _git_stage ()
>  
>  __git_config_get_set_variables ()
>  {
> -	local prevword word config_file= c=$COMP_CWORD
> +	local prevword word config_file= c
> +	local git_comp_words
> +	__reassemble_comp_words_by_ref = git_comp_words c

Use _get_comp_words_by_ref() instead.

>  	while [ $c -gt 1 ]; do
> -		word="${COMP_WORDS[c]}"
> +		word="${git_comp_words[c]}"
>  		case "$word" in
>  		--global|--system|--file=*)
>  			config_file="$word"


> @@ -2045,7 +2327,8 @@ _git_reset ()
>  {
>  	__git_has_doubledash && return
>  
> -	local cur="${COMP_WORDS[COMP_CWORD]}"
> +	local cur
> +_get_comp_words_by_ref cur

Missing indentation.

>  	case "$cur" in
>  	--*)
>  		__gitcomp "--merge --mixed --hard --soft --patch"


> @@ -2343,15 +2634,16 @@ _git_whatchanged ()
>  
>  _git ()
>  {
> -	local i c=1 command __git_dir
> +	local i c=1 command __git_dir git_comp_words git_comp_cword
>  
>  	if [[ -n $ZSH_VERSION ]]; then
>  		emulate -L bash
>  		setopt KSH_TYPESET
>  	fi
>  
> -	while [ $c -lt $COMP_CWORD ]; do
> -		i="${COMP_WORDS[c]}"
> +	__reassemble_comp_words_by_ref = git_comp_words git_comp_cword

Use _get_comp_words_by_ref() instead.

> +	while [ $c -lt $git_comp_cword ]; do
> +		i="${git_comp_words[c]}"
>  		case "$i" in
>  		--git-dir=*) __git_dir="${i#--git-dir=}" ;;
>  		--bare)      __git_dir="." ;;

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2/RFC] Make git-completion Bash 4 compatible.
  2010-10-27 17:31 ` Jonathan Nieder
@ 2010-10-27 22:53   ` SZEDER Gábor
  2010-10-28  0:52     ` Peter van der Does
  0 siblings, 1 reply; 11+ messages in thread
From: SZEDER Gábor @ 2010-10-27 22:53 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Peter van der Does, Shawn O. Pearce, git, Marc Branchaud,
	Brian Gernhardt, Kevin Ballard, Mathias Lafeldt

Hi Jonathan,


On Wed, Oct 27, 2010 at 12:31:32PM -0500, Jonathan Nieder wrote:
> Could some zsh user perhaps test that the new zsh support is not
> broken?

I'm afraid it is.

The commit message of 06f44c3 (completion: make compatible with zsh,
2010-09-06) says:

    ${var:2}
        Zsh does not implement ${var:2} to skip the first 2 characters, but
        ${var#??} works in both shells to replace the first 2 characters
        with nothing.  Thanks to Jonathan Nieder for the suggestion.

    for (( n=1; "$n" ... ))
        Zsh does not allow "$var" in arithmetic loops.  Instead, pre-compute
        the endpoint and use the variables without $'s or quotes.

However, the functions taken over from the bash-completion code
contain constructs like:

    ${cur:0:$index}
    # ok, this is not exactly the same as ${var:2}, so it might even
    # work...

and

    for (( i=0, j=0; i < ${#COMP_WORDS[@]}; i++, j++)); do

But I haven't actually tried it.


Best,
Gábor

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2/RFC] Make git-completion Bash 4 compatible.
  2010-10-27 22:39 ` SZEDER Gábor
@ 2010-10-28  0:48   ` Jonathan Nieder
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Nieder @ 2010-10-28  0:48 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: Peter van der Does, Shawn O. Pearce, git, Marc Branchaud,
	Brian Gernhardt, Kevin Ballard, Mathias Lafeldt

SZEDER Gábor wrote:

> I'm still puzzled that the only relevant entry I could find in the
> bash NEWS file is:
> 
> i.  The programmable completion code now uses the same set of characters as
>     readline when breaking the command line into a list of words.

Here's a note to the Debian bash maintainer on that subject:

 http://bugs.debian.org/601632

I'm still too confused to come up with a documentation patch to send to
bug-bash.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2/RFC] Make git-completion Bash 4 compatible.
  2010-10-27 22:53   ` SZEDER Gábor
@ 2010-10-28  0:52     ` Peter van der Does
  2010-10-28  0:54       ` Jonathan Nieder
  0 siblings, 1 reply; 11+ messages in thread
From: Peter van der Does @ 2010-10-28  0:52 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: git, Jonathan Nieder, Shawn O. Pearce, Marc Branchaud,
	Brian Gernhardt, Kevin Ballard, Mathias Lafeldt

On Thu, 28 Oct 2010 00:53:18 +0200
SZEDER Gábor <szeder@ira.uka.de> wrote:

> Hi Jonathan,
> 
> 
> On Wed, Oct 27, 2010 at 12:31:32PM -0500, Jonathan Nieder wrote:
> > Could some zsh user perhaps test that the new zsh support is not
> > broken?
> 
> I'm afraid it is.
> 
> The commit message of 06f44c3 (completion: make compatible with zsh,
> 2010-09-06) says:
> 
>     ${var:2}
>         Zsh does not implement ${var:2} to skip the first 2
> characters, but ${var#??} works in both shells to replace the first 2
> characters with nothing.  Thanks to Jonathan Nieder for the
> suggestion.
> 
>     for (( n=1; "$n" ... ))
>         Zsh does not allow "$var" in arithmetic loops.  Instead,
> pre-compute the endpoint and use the variables without $'s or quotes.
> 
> However, the functions taken over from the bash-completion code
> contain constructs like:
> 
>     ${cur:0:$index}
>     # ok, this is not exactly the same as ${var:2}, so it might even
>     # work...
> 
> and
> 
>     for (( i=0, j=0; i < ${#COMP_WORDS[@]}; i++, j++)); do
> 
> But I haven't actually tried it.
> 
> 
> Best,
> Gábor
> 
On the zsh change, I replied to the email "What's cooking in git.git
(Oct 2010, #02; Tue, 26)"

> With the patch "Make git-completion Bash 4 compatible" in mind, it
> might be useful to start looking into a different way to distribute
> the completion script to accommodate different shells. Adding
> compatibility for each shell into one script can get nasty. We could
> have a different completion script for each shell.

The bash completion script could still be included with the core, but
we can offer different versions for different shells.

-- 
Peter van der Does

GPG key: E77E8E98

IRC: Ganseki on irc.freenode.net
Twitter: @petervanderdoes

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2/RFC] Make git-completion Bash 4 compatible.
  2010-10-28  0:52     ` Peter van der Does
@ 2010-10-28  0:54       ` Jonathan Nieder
  2010-10-28 12:14         ` Peter van der Does
  0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Nieder @ 2010-10-28  0:54 UTC (permalink / raw)
  To: Peter van der Does
  Cc: SZEDER Gábor, git, Shawn O. Pearce, Marc Branchaud,
	Brian Gernhardt, Kevin Ballard, Mathias Lafeldt

Peter van der Does wrote:

> The bash completion script could still be included with the core, but
> we can offer different versions for different shells.

Why?  That's three times the maintenance work.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2/RFC] Make git-completion Bash 4 compatible.
  2010-10-28  0:54       ` Jonathan Nieder
@ 2010-10-28 12:14         ` Peter van der Does
  2010-10-28 16:15           ` Jakub Narebski
  0 siblings, 1 reply; 11+ messages in thread
From: Peter van der Does @ 2010-10-28 12:14 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: git, SZEDER Gábor, Shawn O. Pearce, Marc Branchaud,
	Brian Gernhardt, Kevin Ballard, Mathias Lafeldt

On Wed, 27 Oct 2010 19:54:55 -0500
Jonathan Nieder <jrnieder@gmail.com> wrote:

> Peter van der Does wrote:
> 
> > The bash completion script could still be included with the core,
> > but we can offer different versions for different shells.
> 
> Why?  That's three times the maintenance work.

The cons of everything in one script:
- If the script needs an update the submitter has to take in account
  the different coding standards each shell has. Examples of this have
  been given in the commit message of 06f44c3 (completion: make
  compatible with zsh, 2010-09-06)

- The script could end up with a slew of if statements to see which
  shell the script is running in and taking some action.

- Shells don't share all the same functions, the script could be filled
  with functions not needed in other shells. the zsh patch includes
  one, the Bash 4 patch includes several.

The pros of everything in one script:
- Small changes, like adding an extra option to a git command for
  completion only have to implemented in one script.


The maintenance of the various scripts would be done by people who have
a full understanding of the shell for which the script is written
for.
I kind of see this suggestion as building the git package for a Linux
distribution, Windows or Mac. Debian and Ubuntu have some patches
that are not included in the core to make git work better on those
distributions. The maintenance is done by the people who have in depth
knowledge of that distribution.


-- 
Peter van der Does

GPG key: E77E8E98

IRC: Ganseki on irc.freenode.net
Twitter: @petervanderdoes

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2/RFC] Make git-completion Bash 4 compatible.
  2010-10-28 12:14         ` Peter van der Does
@ 2010-10-28 16:15           ` Jakub Narebski
  2010-10-28 18:46             ` Peter van der Does
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Narebski @ 2010-10-28 16:15 UTC (permalink / raw)
  To: Peter van der Does
  Cc: Jonathan Nieder, git, SZEDER Gábor, Shawn O. Pearce,
	Marc Branchaud, Brian Gernhardt, Kevin Ballard, Mathias Lafeldt

Peter van der Does <peter@avirtualhome.com> writes:
> On Wed, 27 Oct 2010 19:54:55 -0500
> Jonathan Nieder <jrnieder@gmail.com> wrote:
> 
> > Peter van der Does wrote:
> > 
> > > The bash completion script could still be included with the core,
> > > but we can offer different versions for different shells.
> > 
> > Why?  That's three times the maintenance work.
> 
> The cons of everything in one script:
> - If the script needs an update the submitter has to take in account
>   the different coding standards each shell has. Examples of this have
>   been given in the commit message of 06f44c3 (completion: make
>   compatible with zsh, 2010-09-06)
> 
> - The script could end up with a slew of if statements to see which
>   shell the script is running in and taking some action.
> 
> - Shells don't share all the same functions, the script could be filled
>   with functions not needed in other shells. the zsh patch includes
>   one, the Bash 4 patch includes several.
> 
> The pros of everything in one script:
> - Small changes, like adding an extra option to a git command for
>   completion only have to implemented in one script.

What about having separate scripts, but sourcing common library that
doesn't do completion, but just provides list of possible completions?
This would be best of both worlds, I think.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2/RFC] Make git-completion Bash 4 compatible.
  2010-10-28 16:15           ` Jakub Narebski
@ 2010-10-28 18:46             ` Peter van der Does
  0 siblings, 0 replies; 11+ messages in thread
From: Peter van der Does @ 2010-10-28 18:46 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: Jonathan Nieder, git, SZEDER Gábor, Shawn O. Pearce,
	Marc Branchaud, Brian Gernhardt, Kevin Ballard, Mathias Lafeldt

On Thu, 28 Oct 2010 09:15:45 -0700 (PDT)
Jakub Narebski <jnareb@gmail.com> wrote:

> Peter van der Does <peter@avirtualhome.com> writes:
> > On Wed, 27 Oct 2010 19:54:55 -0500
> > Jonathan Nieder <jrnieder@gmail.com> wrote:
> > 
> > > Peter van der Does wrote:
> > > 
> > > > The bash completion script could still be included with the
> > > > core, but we can offer different versions for different shells.
> > > 
> > > Why?  That's three times the maintenance work.
> > 
> > The cons of everything in one script:
> > - If the script needs an update the submitter has to take in account
> >   the different coding standards each shell has. Examples of this
> > have been given in the commit message of 06f44c3 (completion: make
> >   compatible with zsh, 2010-09-06)
> > 
> > - The script could end up with a slew of if statements to see which
> >   shell the script is running in and taking some action.
> > 
> > - Shells don't share all the same functions, the script could be
> > filled with functions not needed in other shells. the zsh patch
> > includes one, the Bash 4 patch includes several.
> > 
> > The pros of everything in one script:
> > - Small changes, like adding an extra option to a git command for
> >   completion only have to implemented in one script.
> 
> What about having separate scripts, but sourcing common library that
> doesn't do completion, but just provides list of possible completions?
> This would be best of both worlds, I think.
> 

That would be the best solution.

I believe it's very hard to accomplish though.
The options for completion are determined within each completion
function, and depending on the option a decision on what to offer the
user is made.
The way to decide which option is given and what to offer differs, some
functions use for/next loops, some use case <var>.

-- 
Peter van der Does

GPG key: E77E8E98

IRC: Ganseki on irc.freenode.net
Twitter: @petervanderdoes

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2010-10-28 18:47 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-27 17:15 [PATCH v2/RFC] Make git-completion Bash 4 compatible Peter van der Does
2010-10-27 17:23 ` Brian Gernhardt
2010-10-27 17:31 ` Jonathan Nieder
2010-10-27 22:53   ` SZEDER Gábor
2010-10-28  0:52     ` Peter van der Does
2010-10-28  0:54       ` Jonathan Nieder
2010-10-28 12:14         ` Peter van der Does
2010-10-28 16:15           ` Jakub Narebski
2010-10-28 18:46             ` Peter van der Does
2010-10-27 22:39 ` SZEDER Gábor
2010-10-28  0:48   ` Jonathan Nieder

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.