Separate "__bp_invoke_pre{cmd,exec}_functions"

* Do not prefix local varnames with underscores
* Make "__bp_invoke_pre{cmd,exec}_functions" return the last non-zero exit status
* Test "__bp_invoke_pre{cmd,exec}_functions"

Author: akinomyoga

bash-preexec.sh

@@ -157,21 +157,38 @@ __bp_precmd_invoke_cmd() {
         return
     fi
     local __bp_inside_precmd=1
+    __bp_invoke_precmd_functions "$__bp_last_ret_value" "$__bp_last_argument_prev_command"
 
+    __bp_set_ret_value "$__bp_last_ret_value" "$__bp_last_argument_prev_command"
+}
+
+# This function invokes every function defined in our function array
+# "precmd_function".  This function receives the arguments $1 and $2 for $? and
+# $_, respectively, which will be set for each precmd function. This function
+# returns the last non-zero exit status of the hook functions. If there is no
+# error, this function returns 0.
+__bp_invoke_precmd_functions() {
+    local lastexit=$1 lastarg=$2
     # Invoke every function defined in our function array.
     local precmd_function
+    local precmd_function_ret_value
+    local precmd_ret_value=0
     for precmd_function in "${precmd_functions[@]}"; do
 
         # Only execute this function if it actually exists.
         # Test existence of functions with: declare -[Ff]
         if type -t "$precmd_function" 1>/dev/null; then
-            __bp_set_ret_value "$__bp_last_ret_value" "$__bp_last_argument_prev_command"
+            __bp_set_ret_value "$lastexit" "$lastarg"
             # Quote our function invocation to prevent issues with IFS
             "$precmd_function"
+            precmd_function_ret_value=$?
+            if [[ "$precmd_function_ret_value" != 0 ]]; then
+                precmd_ret_value="$precmd_function_ret_value"
+            fi
         fi
     done
 
-    __bp_set_ret_value "$__bp_last_ret_value"
+    __bp_set_ret_value "$precmd_ret_value"
 }
 
 # Sets a return value in $?. We may want to get access to the $? variable in our
@@ -258,7 +275,27 @@ __bp_preexec_invoke_exec() {
         return
     fi
 
-    # Invoke every function defined in our function array.
+    __bp_invoke_preexec_functions "${__bp_last_ret_value:-}" "$__bp_last_argument_prev_command" "$this_command"
+    local preexec_ret_value=$?
+
+    # Restore the last argument of the last executed command, and set the return
+    # value of the DEBUG trap to be the return code of the last preexec function
+    # to return an error.
+    # If `extdebug` is enabled a non-zero return value from any preexec function
+    # will cause the user's command not to execute.
+    # Run `shopt -s extdebug` to enable
+    __bp_set_ret_value "$preexec_ret_value" "$__bp_last_argument_prev_command"
+}
+
+# This function invokes every function defined in our function array
+# "preexec_function".  This function receives the arguments $1 and $2 for $?
+# and $_, respectively, which will be set for each preexec function.  The third
+# argument $3 specifies the user command that is going to be executed
+# (corresponding to BASH_COMMAND in the DEBUG trap).  This function returns the
+# last non-zero exit status from the preexec functions.  If there is no error,
+# this function returns `0`.
+__bp_invoke_preexec_functions() {
+    local lastexit=$1 lastarg=$2 this_command=$3
     local preexec_function
     local preexec_function_ret_value
     local preexec_ret_value=0
@@ -267,7 +304,7 @@ __bp_preexec_invoke_exec() {
         # Only execute each function if it actually exists.
         # Test existence of function with: declare -[fF]
         if type -t "$preexec_function" 1>/dev/null; then
-            __bp_set_ret_value "${__bp_last_ret_value:-}"
+            __bp_set_ret_value "$lastexit" "$lastarg"
             # Quote our function invocation to prevent issues with IFS
             "$preexec_function" "$this_command"
             preexec_function_ret_value="$?"
@@ -276,14 +313,7 @@ __bp_preexec_invoke_exec() {
             fi
         fi
     done
-
-    # Restore the last argument of the last executed command, and set the return
-    # value of the DEBUG trap to be the return code of the last preexec function
-    # to return an error.
-    # If `extdebug` is enabled a non-zero return value from any preexec function
-    # will cause the user's command not to execute.
-    # Run `shopt -s extdebug` to enable
-    __bp_set_ret_value "$preexec_ret_value" "$__bp_last_argument_prev_command"
+    __bp_set_ret_value "$preexec_ret_value"
 }
 
 __bp_install() {

test/bash-preexec.bats

@@ -328,6 +328,76 @@ set_exit_code_and_run_precmd() {
     [ $status -eq 1 ]
 }
 
+@test "__bp_invoke_precmd_functions should be transparent for \$? and \$_" {
+  tester1() { test1_lastexit=$? test1_lastarg=$_; }
+  tester2() { test2_lastexit=$? test2_lastarg=$_; }
+  precmd_functions=(tester1 tester2)
+  trap - DEBUG # remove the Bats stack-trace trap so $_ doesn't get overwritten
+  __bp_invoke_precmd_functions 111 'vxxJlwNx9VPJDA' || true
+
+  [ "$test1_lastexit" == 111 ]
+  [ "$test1_lastarg" == 'vxxJlwNx9VPJDA' ]
+  [ "$test2_lastexit" == 111 ]
+  [ "$test2_lastarg" == 'vxxJlwNx9VPJDA' ]
+}
+
+@test "__bp_invoke_precmd_functions returns the last non-zero exit status" {
+  tester1() { return 91; }
+  tester2() { return 38; }
+  tester3() { return 0; }
+  precmd_functions=(tester1 tester2 tester3)
+  status=0
+  __bp_invoke_precmd_functions 1 'lastarg' || status=$?
+
+  [ "$status" == 38 ]
+
+  precmd_functions=(tester3)
+  status=0
+  __bp_invoke_precmd_functions 1 'lastarg' || status=$?
+
+  [ "$status" == 0 ]
+}
+
+@test "__bp_invoke_preexec_functions should be transparent for \$? and \$_" {
+  tester1() { test1_lastexit=$? test1_lastarg=$_; }
+  tester2() { test2_lastexit=$? test2_lastarg=$_; }
+  preexec_functions=(tester1 tester2)
+  trap - DEBUG # remove the Bats stack-trace trap so $_ doesn't get overwritten
+  __bp_invoke_preexec_functions 87 'ehQrzHTHtE2E7Q' 'command' || true
+
+  [ "$test1_lastexit" == 87 ]
+  [ "$test1_lastarg" == 'ehQrzHTHtE2E7Q' ]
+  [ "$test2_lastexit" == 87 ]
+  [ "$test2_lastarg" == 'ehQrzHTHtE2E7Q' ]
+}
+
+@test "__bp_invoke_preexec_functions returns the last non-zero exit status" {
+  tester1() { return 52; }
+  tester2() { return 112; }
+  tester3() { return 0; }
+  preexec_functions=(tester1 tester2 tester3)
+  status=0
+  __bp_invoke_preexec_functions 1 'lastarg' 'command' || status=$?
+
+  [ "$status" == 112 ]
+
+  preexec_functions=(tester3)
+  status=0
+  __bp_invoke_preexec_functions 1 'lastarg' 'command' || status=$?
+
+  [ "$status" == 0 ]
+}
+
+@test "__bp_invoke_preexec_functions should supply a current command in the first argument" {
+  tester1() { test1_bash_command=$1; }
+  tester2() { test2_bash_command=$1; }
+  preexec_functions=(tester1 tester2)
+  __bp_invoke_preexec_functions 1 'lastarg' 'UEVkErELArSwjA' || true
+
+  [ "$test1_bash_command" == 'UEVkErELArSwjA' ]
+  [ "$test2_bash_command" == 'UEVkErELArSwjA' ]
+}
+
 @test "in_prompt_command should detect if a command is part of PROMPT_COMMAND" {
 
     PROMPT_COMMAND=$'precmd_invoke_cmd\n something; echo yo\n __bp_interactive_mode'