Minor fixes identified through shellcheck and other tools (#350)

* fix whitespace

* fix spelling

* fix whitespace

* unify headers of tests

* fix some issues in tests, identified by shellcheck

* fix bash completions

bash completion files are not supposed to be executable

* fix some issues identified by shellcheck

Co-authored-by: Ali Karbassi <ali@karbassi.com>

tests/README

@@ -18,14 +18,14 @@ The easiest way to run tests is to say "make test" from the top-level.
 This runs all the tests.
 
     rm -rf tests/test-results "tests/trash directory"*
-    cd tests && sh t0000-config.sh 
+    cd tests && sh t0000-config.sh
     *   ok 1: no config file
     *   ok 2: config file (default location 1)
     *   ok 3: config file (default location 2)
     *   ok 4: config file (command line)
     *   ok 5: config file (env variable)
     * passed all 5 test(s)
-    cd tests && sh t0001-null.sh 
+    cd tests && sh t0001-null.sh
     *   ok 1: null ls
     * passed all 1 test(s)
     rm -rf tests/test-results
@@ -33,7 +33,7 @@ This runs all the tests.
 Or you can run each test individually from command line, like
 this:
 
-    $ ./t0001-null.sh 
+    $ ./t0001-null.sh
     *   ok 1: null ls
     * passed all 1 test(s)
 
@@ -42,27 +42,27 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate
 appropriately before running "make".
 
 --verbose::
-	This makes the test more verbose.  Specifically, the
-	command being run and their output if any are also
-	output.
+    This makes the test more verbose.  Specifically, the
+    command being run and their output if any are also
+    output.
 
 --debug::
-	This may help the person who is developing a new test.
-	It causes the command defined with test_debug to run.
+    This may help the person who is developing a new test.
+    It causes the command defined with test_debug to run.
 
 --immediate::
-	This causes the test to immediately exit upon the first
-	failed test.
+    This causes the test to immediately exit upon the first
+    failed test.
 
 --long-tests::
-	This causes additional long-running tests to be run (where
-	available), for more exhaustive testing.
+    This causes additional long-running tests to be run (where
+    available), for more exhaustive testing.
 
 --tee::
-	In addition to printing the test output to the terminal,
-	write it to files named 't/test-results/$TEST_NAME.out'.
-	As the names depend on the tests' file names, it is safe to
-	run the tests with this option in parallel.
+    In addition to printing the test output to the terminal,
+    write it to files named 't/test-results/$TEST_NAME.out'.
+    As the names depend on the tests' file names, it is safe to
+    run the tests with this option in parallel.
 
 Skipping Tests
 --------------
@@ -97,15 +97,15 @@ Naming Tests
 
 The test files are named as:
 
-	tNNNN-commandname-details.sh
+    tNNNN-commandname-details.sh
 
 where N is a decimal digit.
 
 First digit tells the family:
 
-	0 - the absolute basics and global stuff
-	1 - basic every-day usage
-	2 - add ins
+    0 - the absolute basics and global stuff
+    1 - basic every-day usage
+    2 - add ins
 
 Second digit tells the particular command we are testing.
 
@@ -128,15 +128,15 @@ The test script is written as a shell script.  It should start
 with the standard "#!/bin/bash" with copyright notices, and an
 assignment to variable 'test_description', like this:
 
-	#!/bin/bash
-	#
-	# Copyright (c) 2005 Junio C Hamano
-	#
+    #!/bin/bash
+    #
+    # Copyright (c) 2005 Junio C Hamano
+    #
 
-	test_description='xxx test (option --frotz)
+    test_description='xxx test (option --frotz)
 
-	This test registers the following structure in the cache
-	and tries to run git-ls-files with option --frotz.'
+    This test registers the following structure in the cache
+    and tries to run git-ls-files with option --frotz.'
 
 
 Source 'test-lib.sh'
@@ -145,7 +145,7 @@ Source 'test-lib.sh'
 After assigning test_description, the test script should source
 test-lib.sh like this:
 
-	. ./test-lib.sh
+    . ./test-lib.sh
 
 This test harness library does the following things:
 
@@ -219,9 +219,9 @@ library for your script to use.
 
    Example:
 
-	test_expect_success \
-	    'git-write-tree should be able to write an empty tree.' \
-	    'tree=$(git-write-tree)'
+    test_expect_success \
+        'git-write-tree should be able to write an empty tree.' \
+        'tree=$(git-write-tree)'
 
  - test_expect_code <code> <message> <script>
 
@@ -266,7 +266,7 @@ and the dates and timestamps will be artificially fixed.
 Then the session can be used to make a unit test thanks to
 test_todo_session, see the existing tests as examples.
 
-Be careful to replace all occurences of the full path to the test
+Be careful to replace all occurrences of the full path to the test
 directory by $HOME as testshell.sh will explain you when you execute it
 otherwise the tests will work properly only on your own computer.
 

tests/actions-test-lib.sh

@@ -21,7 +21,7 @@ make_action_in_folder()
 {
     unset TODO_ACTIONS_DIR
     [ -d .todo.actions.d ] || mkdir .todo.actions.d
-    mkdir .todo.actions.d/$1
+    mkdir ".todo.actions.d/$1"
     cat > ".todo.actions.d/$1/$1" <<EOF
 #!/bin/bash
 [ "\$1" = "usage" ] && {

tests/aggregate-results.sh

@@ -15,7 +15,7 @@ case "$1" in
 esac
 
 if test -n "$color"; then
-	say_color () {
+	say_color() {
 		(
 		export TERM
 		case "$1" in
@@ -67,15 +67,15 @@ do
 		'')
 			continue ;;
 		fixed)
-			fixed=$(($fixed + $value)) ;;
+			fixed=$((fixed + $value)) ;;
 		success)
-			success=$(($success + $value)) ;;
+			success=$((success + $value)) ;;
 		failed)
-			failed=$(($failed + $value)) ;;
+			failed=$((failed + $value)) ;;
 		broken)
-			broken=$(($broken + $value)) ;;
+			broken=$((broken + $value)) ;;
 		total)
-			total=$(($total + $value)) ;;
+			total=$((total + $value)) ;;
 		esac
 	done <"$file"
 done

tests/t0000-config.sh

@@ -20,12 +20,12 @@ test_expect_success 'no config file' '
 '
 
 # All the below tests will output the usage message.
-cat > expect << EOF
+cat > expect <<EOF
 Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description]
 Try 'todo.sh -h' for more information.
 EOF
 
-cat > test.cfg << EOF
+cat > test.cfg <<EOF
 export TODO_DIR=.
 export TODO_FILE="\$TODO_DIR/todo.txt"
 export DONE_FILE="\$TODO_DIR/done.txt"
@@ -79,7 +79,7 @@ test_expect_success 'config file (env variable)' '
     test_cmp expect output && test -f used_config
 '
 
-cat > minimal.cfg << EOF
+cat > minimal.cfg <<EOF
 export TODO_DIR=.
 touch used_config
 EOF

tests/t0001-null.sh

@@ -59,7 +59,6 @@ test_expect_success 'null list filter' '
     todo.sh listall filter > output && test_cmp expect output
 '
 
-
 #
 # lsc|listcon
 #

tests/t0002-actions.sh

@@ -7,11 +7,11 @@ This test just makes sure that todo.sh can locate custom actions.
 . ./test-lib.sh
 
 # All the below tests will output the custom action message
-cat > expect << EOF
+cat > expect <<EOF
 TODO: foo
 EOF
 
-cat > foo << EOF
+cat > foo <<EOF
 echo "TODO: foo"
 EOF
 chmod +x foo

tests/t1020-addtolistfile.sh

@@ -66,7 +66,7 @@ test_todo_session 'basic listfile filtering' <<EOF
 --
 GARDEN: 1 of 2 tasks shown
 
->>> todo.sh listfile garden.txt smell  
+>>> todo.sh listfile garden.txt smell
 2 smell the roses
 --
 GARDEN: 1 of 2 tasks shown
@@ -89,7 +89,7 @@ test_todo_session 'addto with &' <<EOF
 4 dig the garden & water the flowers
 GARDEN: 4 added.
 
->>> todo.sh listfile garden.txt 
+>>> todo.sh listfile garden.txt
 4 dig the garden & water the flowers
 1 notice the daisies
 2 smell the roses

tests/t1100-replace.sh

@@ -45,7 +45,7 @@ grow some corn
 thrash some hay
 chase the chickens
 EOF
-test_todo_session 'replace error' << EOF
+test_todo_session 'replace error' <<EOF
 >>> todo.sh replace 10 "hej!"
 === 1
 TODO: No task 10.
@@ -82,7 +82,7 @@ TODO: Replaced task with:
 EOF
 
 echo 'jump on hay' > todo.txt
-test_todo_session 'replace with &' << EOF
+test_todo_session 'replace with &' <<EOF
 >>> todo.sh replace 1 "thrash the hay & thrash the wheat"
 1 jump on hay
 TODO: Replaced task with:

tests/t1300-ls.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='list functionality
 
@@ -162,7 +161,6 @@ TODO: 1 of 5 tasks shown
 TODO: 1 of 5 tasks shown
 EOF
 
-
 #
 # check the x command line option
 #

tests/t1310-listcon.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='listcon functionality
 

tests/t1320-listproj.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='listproj functionality
 
@@ -117,7 +116,6 @@ test_todo_session 'listproj with context special cases' <<EOF
 +sunflowers
 EOF
 
-
 cat > todo.txt <<EOF
 +prj01 -- Some project 1 task
 EOF

tests/t1330-ls-highlighting.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='list highlighting
 

tests/t1340-listescapes.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='list with escape sequences
 

tests/t1360-ls-project-context-highlighting.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='highlighting projects and contexts
 

tests/t1380-ls-date-number-metadata-highlighting.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='highlighting date, item numbers and metadata
 
@@ -32,7 +31,6 @@ test_todo_session 'highlighting for date, item numbers and metadata' <<'EOF'
 TODO: 3 of 3 tasks shown
 EOF
 
-
 test_todo_session 'suppressing highlighting for date, item numbers and metadata' <<'EOF'
 >>> todo.sh -p -d "$TEST_TODO_LABEL_COLORS" ls
 1 2018-11-11 task with date

tests/t1500-do.sh

@@ -83,4 +83,5 @@ TODO: 3 marked as done.
 >>> todo.sh -a do 3
 TODO: 3 is already marked done.
 EOF
+
 test_done

tests/t1600-append.sh

@@ -17,7 +17,7 @@ test_todo_session 'append usage' <<EOF
 usage: todo.sh append ITEM# "TEXT TO APPEND"
 EOF
 
-test_todo_session 'append error' << EOF
+test_todo_session 'append error' <<EOF
 >>> todo.sh append 10 "hej!"
 === 1
 TODO: No task 10.

tests/t1850-move.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='basic move functionality
 '

tests/t2100-help.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='help functionality
 

tests/t2120-shorthelp.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='shorthelp functionality
 
@@ -36,7 +35,6 @@ test_todo_session 'shorthelp output with custom action' <<EOF
   See "help" for more details.
 EOF
 
-
 # Verify that custom configuration is actually processed (when the -d option
 # precedes the -h option) by specifying a different actions directory and moving
 # our custom action there. The help output should mention the "Add-On Actions".

tests/t2200-no-done-report-files.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='todo.sh configuration with a sole todo.txt data file.
 
@@ -8,7 +7,7 @@ checks that no such empty files are created.
 '
 . ./test-lib.sh
 
-cat > test.cfg << EOF
+cat > test.cfg <<EOF
 export TODO_DIR=.
 export TODO_FILE="\$TODO_DIR/todo.txt"
 export DONE_FILE=/dev/null

tests/t6000-completion.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='Bash completion functionality
 

tests/t6010-completion-contexts.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='Bash context completion functionality
 

tests/t6020-completion-projects.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='Bash project completion functionality
 

tests/t6030-completion-tasks.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='Bash task number completion functionality
 

tests/t6040-completion-files.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='Bash todo file completion functionality
 

tests/t6050-completion-addons.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='Bash add-on action completion functionality
 
@@ -74,7 +73,7 @@ removeCustomActions "$HOME/.todo.actions.d"
 #
 # Test resolution of an alternative TODO_ACTIONS_DIR.
 #
-mkdir  "$HOME/.todo"
+mkdir "$HOME/.todo"
 makeCustomActions "$HOME/.todo/actions"
 test_todo_completion 'all arguments with actions from .todo/actions/' 'todo.sh ' "$ACTIONS $ADDONS $CONTAINED $OPTIONS"
 removeCustomActions "$HOME/.todo/actions"

tests/t6060-completion-addon-files.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='Bash add-on action file completion functionality
 

tests/t6080-completion-path.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='Bash completion with different path functionality
 
@@ -28,8 +27,6 @@ _todo2()
 
 test_todo_custom_completion _todo2 'all todo2 contexts' 'todo2 list @' '@garden @outdoor @outside'
 
-
-
 # Remove the test environment's bin directory from the PATH, so that our test
 # executable must be launched with an explicit path.
 PATH=${PATH##"${PWD}/bin:"}

tests/t6090-completion-aliases.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='Bash completion with different aliases functionality
 
@@ -46,7 +45,6 @@ test_todo_session 'todo 1 and 2 contexts' <<EOF
 @oriental
 EOF
 
-
 # Define a second completion function that injects the different configuration
 # file. In real use, this would be installed via
 #   complete -F _todo2 todo2

tests/t6100-completion-help.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 test_description='Bash help completion functionality
 

tests/test-lib.sh

@@ -172,12 +172,12 @@ test_set_editor () {
 # the text_expect_* functions instead.
 
 test_ok_ () {
-	test_success=$(($test_success + 1))
+	test_success=$((test_success + 1))
 	say_color "" "  ok $test_count: $@"
 }
 
 test_failure_ () {
-	test_failure=$(($test_failure + 1))
+	test_failure=$((test_failure + 1))
 	say_color error "FAIL $test_count: $1"
 	shift
 	echo "$@"
@@ -185,12 +185,12 @@ test_failure_ () {
 }
 
 test_known_broken_ok_ () {
-	test_fixed=$(($test_fixed+1))
+	test_fixed=$((test_fixed + 1))
 	say_color "" "  FIXED $test_count: $@"
 }
 
 test_known_broken_failure_ () {
-	test_broken=$(($test_broken+1))
+	test_broken=$((test_broken + 1))
 	say_color skip "  still broken $test_count: $@"
 }
 
@@ -206,7 +206,7 @@ test_run_ () {
 }
 
 test_skip () {
-	test_count=$(($test_count+1))
+	test_count=$((test_count + 1))
 	to_skip=
 	for skp in $SKIP_TESTS
 	do
@@ -364,7 +364,7 @@ test_external_without_stderr () {
 		test_ok_ "$descr"
 	else
 		if [ "$verbose" = t ]; then
-			output=`echo; echo Stderr is:; cat "$stderr"`
+			output=$(echo; echo Stderr is:; cat "$stderr")
 		else
 			output=
 		fi
@@ -428,7 +428,7 @@ test_done () {
 	if test "$test_broken" != 0
 	then
 		say_color error "still have $test_broken known breakage(s)"
-		msg="remaining $(($test_count-$test_broken)) test(s)"
+		msg="remaining $((test_count - test_broken)) test(s)"
 	else
 		msg="$test_count test(s)"
 	fi
@@ -471,7 +471,7 @@ rm -fr "$test" || {
 test_init_todo () {
 	test "$#" = 1 ||
 	error "bug in the test script: not 1 parameter to test_init_todo"
-	owd=`pwd`
+	owd=$(pwd)
 	root="$1"
 	mkdir -p "$root"
 	cd "$root" || error "Cannot setup todo dir in $root"
@@ -548,7 +548,7 @@ test_init_todo () {
 
 # Usage: test_tick [increment]
 test_tick () {
-	TODO_TEST_TIME=$(($TODO_TEST_TIME + ${1:-86400}))
+	TODO_TEST_TIME=$((TODO_TEST_TIME + ${1:-86400}))
 }
 
 # Generate and run a series of tests based on a transcript.
@@ -581,14 +581,14 @@ test_todo_session () {
 	    status=${line#=== }
 	    ;;
 	"")
-	    if [ ! -z "$cmd" ]; then
-		if [ $status = 0 ]; then
+	    if [ -n "$cmd" ]; then
+		if [ "$status" = 0 ]; then
 		    test_expect_output "$1 $subnum" "$cmd"
 		else
 		    test_expect_code_and_output "$status" "$1 $subnum" "$cmd"
 		fi
 
-		subnum=$(($subnum + 1))
+		subnum=$((subnum + 1))
 		cmd=""
 		status=0
 		> expect
@@ -602,8 +602,8 @@ test_todo_session () {
 	    ;;
 	esac
     done
-    if [ ! -z "$cmd" ]; then
-	if [ $status = 0 ]; then
+    if [ -n "$cmd" ]; then
+	if [ "$status" = 0 ]; then
 	    test_expect_output "$1 $subnum" "$cmd"
 	else
 	    test_expect_code_and_output "$status" "$1 $subnum" "$cmd"
@@ -645,7 +645,7 @@ test_todo_custom_completion () {
 		SAVEIFS=$IFS
 		IFS=' ' set -- $2
 		COMP_WORDS=("$@")
-		COMP_CWORD=$(($# - $offset))
+		COMP_CWORD=$(($# - offset))
 		IFS=' ' eval "set -- $expected"
 		EXPECT=("$@")