[PATCH] bash_completion: improvement, fixes and tests [was: Re: subversion and programmable completion]

[Julian Foad <julianfoad@xxxxxxxxxxxxxxx>]

Sebastien Cevey wrote:
> The bash completion does not use the content of 'svn help'
> AFAIK. (we're talking about the script in tools/client-side/ right?).
...
> I had not thought of using svn help output to feed the completion
> though, that sounds quite a neat idea. Are you willing to work on it
> (you seemed to use zsh rather than bash though) ? Or do you know
> existing examples of such bash scripts that could be a basis for an
> svn one ?

Here is a step towards that.

After I fixed a trivial bug in bash_completion and added recognition of "--arg=value", I wrote a test script that (among other tests) checks the completions against "svn help".  That showed me lots more bugs.  So here are the fixes and the test script (which is not run automatically by "make check").

Note: the test script still fails for one reason: "--version" is described by "svn help" as an option to "svn help".  That is wrong: it doesn't work like that, though it is sort of internally handled that way.  "--version" and "--help" and "-h" are (should be) options to "svn" (without a subcommand).  I regard this as a bug in "svn", not in "bash_completion" or "bash_completion_test".

I could adjust bash_completion and/or bash_completion_test to overlook this anomoly if you really want.  It should be marked "XFAIL" but that's not easy to do.  This is what it says:

~/src/subversion/tools/client-side> ./bash_completion_test ./bash_completion
Checking general completion
Checking list of subcommands
Checking list of options for each subcommand
FAIL: completions for "svn ? -" != options accepted
         (completions: --quiet -q )
         (svn accepts: --quiet --version -q )
FAIL: completions for "svn h -" != options accepted
         (completions: --quiet -q )
         (svn accepts: --quiet --version -q )
FAIL: completions for "svn help -" != options accepted
         (completions: --quiet -q )
         (svn accepts: --quiet --version -q )
Checking rejection of synonyms
FAILURE: at least one bash_completion test failed.


Is this acceptable the way it is, given that the test script is only to be run manually by someone who is interested in bash_completion?

- Julian


Log message:
Improve and fix bash_completion.
Add a script that tests bash_completion, including checking the completion
results against the output of "svn help [...]".

* tools/client-side/bash_completion
 (_svn) Recognise "--arg=value" as an alternative to "--arg value" (when
        excluding options or synonyms that are already present).
        Fix the list of subcommands and options accepted as a first argument.
        Fix the lists of options accepted by various subcommands.

* tools/client-side/bash_completion_test
 New file: a shell script that tests bash_completion.


Index: tools/client-side/bash_completion
===================================================================
--- tools/client-side/bash_completion	(revision 6521)
+++ tools/client-side/bash_completion	(working copy)
@@ -9,26 +9,26 @@
 
 _svn()
 {
-	local cur cmds cmdOpts pOpts mOpts rOpts qOpts nOpts optsParam opt
+	local cur cmds cmdOpts pOpts mOpts rOpts qOpts nOpts optsParam opt optBase
 
 	COMPREPLY=()
 	cur=${COMP_WORDS[COMP_CWORD]}
 
 	cmds='add cat checkout co cleanup commit ci copy cp delete del \
-	      remove rm diff di export import info h help list ls log merge \
+	      remove rm diff di export import info ? h help list ls log merge \
 	      mkdir move mv rename ren propdel pdel propedit pedit pe propget \
 	      pget pg proplist plist pl propset pset ps revert resolve status \
-	      stat st switch sw update up --version'
+	      stat st switch sw update up'
 
 	if [[ $COMP_CWORD -eq 1 ]] ; then
-		COMPREPLY=( $( compgen -W "$cmds" -- $cur ) )
+		COMPREPLY=( $( compgen -W "$cmds -h --help --version" -- $cur ) )
 		return 0
 	else
 		# if we're completing for 'svn help' or 'svn h', then just 
 		# complete on any valid svn command
 		case ${COMP_WORDS[1]} in
 		help|h|\?)
-			COMPREPLY=( $( compgen -W "$cmds" -- $cur ) )
+			COMPREPLY=( $( compgen -W "$cmds -q --quiet" -- $cur ) )
 			return 0
 			;;
 		*)
@@ -62,7 +62,7 @@
 		cmdOpts="$qOpts"
 		;;
 	add)
-		cmdOpts="--targets -$nOpts $qOpts"
+		cmdOpts="--targets $nOpts $qOpts"
 		;;
 	cat)
 		cmdOpts="$rOpts $pOpts"
@@ -89,7 +89,7 @@
 		cmdOpts="$rOpts $qOpts $pOpts --force"
 		;;
 	import)
-		cmdOpts="$mOpts $qOpts $nOpts --targets --editor-cmd $pOpts"
+		cmdOpts="$mOpts $qOpts $nOpts --editor-cmd $pOpts"
 		;; 
 	info)
 		cmdOpts="--targets -R --recursive"
@@ -99,7 +99,7 @@
 		;;
 	log)
 		cmdOpts="$rOpts -v --verbose --targets $pOpts --strict \
-		         --incremental --xml"
+		         --incremental --xml $qOpts"
 		;;
 	merge)
 		cmdOpts="$rOpts $nOpts $qOpts --force --dry-run --diff3-cmd \
@@ -115,7 +115,8 @@
 		cmdOpts="$qOpts -R --recursive $rOpts --revprop $pOpts"
 		;;
 	propedit|pedit|pe)
-		cmdOpts="$rOpts --revprop --encoding --editor-cmd $pOpts"
+		cmdOpts="$rOpts --revprop --encoding --editor-cmd $pOpts \
+		         --force"
 		;;
 	propget|pget|pg)
 		cmdOpts="-R --recursive $rOpts --revprop --strict $pOpts"
@@ -126,7 +127,7 @@
 		;;
 	propset|pset|ps)
 		cmdOpts="-F --file $qOpts --targets -R --recursive --revprop \
-		         --encoding $pOpts"
+		         --encoding $pOpts $rOpts --force"
 		;;
 	revert)
 		cmdOpts="--targets -R --recursive $qOpts"
@@ -139,10 +140,10 @@
 		         --no-ignore"
 		;;
 	switch|sw)
-		cmdOpts="--relocate $rOpts $nOpts $qOpts $pOpts"
+		cmdOpts="--relocate $rOpts $nOpts $qOpts $pOpts --diff3-cmd"
 		;;
 	update|up)
-		cmdOpts="$rOpts $nOpts $qOpts $pOpts"
+		cmdOpts="$rOpts $nOpts $qOpts $pOpts --diff3-cmd"
 		;;
 	*)
 		;;
@@ -151,11 +152,17 @@
 	# take out options already given
 	for (( i=2; i<=$COMP_CWORD-1; ++i )) ; do
 		opt=${COMP_WORDS[$i]}
+
+		case $opt in
+		--*)    optBase=${opt/=*/} ;;
+		-*)     optBase=${opt} ;;
+		esac
+
 		cmdOpts=" $cmdOpts "
-		cmdOpts=${cmdOpts/ ${opt} / }
+		cmdOpts=${cmdOpts/ ${optBase} / }
 
 		# take out alternatives
-		case $opt in
+		case $optBase in
 		-v)              cmdOpts=${cmdOpts/ --verbose / } ;;
 		--verbose)       cmdOpts=${cmdOpts/ -v / } ;;
 		-N)              cmdOpts=${cmdOpts/ --non-recursive / } ;;

#!/bin/bash
# Checks that the "_svn" function defined in the specified "bash_completion"
# script produces appropriate lists of completions for various incomplete svn
# command lines.

if [ ! -f "$1" ] || [ "$2" ]; then
  echo "Usage: bash_completion_test BASH_COMPLETION_PATHNAME"
  echo "Tests the specified \"bash_completion\" script,"
  echo "including checking it against the \"svn\" program found in the current PATH."
  exit 1
fi

set -e  # Exit on error
shopt -s extglob

# Execute the script which is to be tested.
. "$1"

# From the given incomplete svn command, print a space-separated list of
# possible completions of the last argument (or of an empty first argument
# if no subcommand is given).
# Usage: get_svn_completions [SVN-SUBCOMMAND [SVN-OPTION...]]
get_svn_completions() {
  COMP_WORDS=(svn "$@")
  if [ $# == 0 ]; then
    COMP_CWORD=1
  else
    COMP_CWORD=$#
  fi
  _svn
  echo -n "${COMPREPLY[*]}"
}

# Print a failure message, record the failure, and return "false".
# Usage: fail MESSAGE
fail() {
  PREFIX="FAIL: "
  for LINE in "$@"; do
    echo "$PREFIX$LINE"
    PREFIX="      "
  done
  TESTS_FAILED=1
  false
}

# Check that EXPECTED-WORD is among the completions of the last word in
# SVN-COMMAND.  SVN-COMMAND is a single argument to this function, split
# into multiple arguments when passed to "get_svn_completions()".
# Usage: includes SVN-COMMAND EXPECTED-WORD
includes() {
  COMPLETIONS=`get_svn_completions $1`
  if [[ "$2" != @(${COMPLETIONS// /|}) ]]; then
    fail "completions of \"svn $1\" should include \"$2\"" \
      "(completions: $COMPLETIONS)"
  fi
}

excludes() {
  COMPLETIONS=`get_svn_completions $1`
  if [[ "$2" == @(${COMPLETIONS// /|}) ]]; then
    fail "completions of \"svn $1\" should exclude \"$2\"" \
      "(completions: $COMPLETIONS)"
  fi
}

# Print the valid subcommands for "svn", one per line, sorted.
# Usage: get_svn_subcommands
get_svn_subcommands() {
  svn help |
    # Find the relevant lines;
    # remove brackets and commas; put each word on its own line.
    sed -n -e '1,/^Available subcommands:$/d;/^$/q' \
           -e 's/[ )]//g;s/[(,]/\n/g;p' |
    sort
}

# Print the valid option switches for "svn SUBCMD", one per line, sorted.
# Usage: get_svn_options SUBCMD
get_svn_options() {
  { svn help "$1" |
      # Find the relevant lines;
      # remove brackets, "arg" and description; put each word on its own line.
      sed -n -e '1,/^Valid options:$/d;/^  -/!d' \
             -e 's/\( arg\)* * : .*//;s/[] ]//g;s/[[]/\n/g;p'
    # The following options are always accepted but not listed in the help,
    # nor currently offered as completions.
    #echo "-h"
    #echo "--help"
  } | sort
  
}


# The tests.
set +e  # Do not exit on error
TESTS_FAILED=

echo "Checking general completion"
includes "he" "help"
includes "" "?"
includes "" "h"
includes "" "help"
includes "" "-h"
includes "" "--help"
includes "" "--version"

echo "Checking list of subcommands"
HELP_SUBCMDS=`get_svn_subcommands | tr "\n" " "`
COMPLETION_SUBCMDS=`get_svn_completions | tr " " "\n" | grep -v "^-" | sort | tr "\n" " "`
if [ "$HELP_SUBCMDS" != "$COMPLETION_SUBCMDS" ]; then
  fail "non-option completions for \"svn \" != subcommands accepted" \
       "    (non-o. cmpl.: $COMPLETION_SUBCMDS)" \
       "    (svn accepts:  $HELP_SUBCMDS)"
fi

echo "Checking list of options for each subcommand"
for SUBCMD in $HELP_SUBCMDS; do
  HELP_OPTIONS=`get_svn_options $SUBCMD | tr "\n" " "`
  COMPLETION_OPTIONS=`get_svn_completions $SUBCMD - | tr " " "\n" | sort | tr "\n" " "`
  if [ "$HELP_OPTIONS" != "$COMPLETION_OPTIONS" ]; then
    fail "completions for \"svn $SUBCMD -\" != options accepted" \
	 "    (completions: $COMPLETION_OPTIONS)" \
         "    (svn accepts: $HELP_OPTIONS)"
  fi
done

echo "Checking rejection of synonyms"
excludes "diff -x -u -" "-x"
excludes "diff -x -u --e" "--extensions"
excludes "diff --extensions -u -" "--extensions"
excludes "diff --extensions -u -" "-x"
excludes "diff --extensions=-u -" "-x"

if [ $TESTS_FAILED ]; then
  echo "FAILURE: at least one bash_completion test failed."
else
  echo "All bash_completion tests passed."
fi