From befc7029576351cc88dfefa334583c0d92650cac Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 22 Jan 2022 16:31:28 +0900 Subject: [PATCH 1/2] 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" | --- bash-preexec.sh | 54 ++++++++++++++++++++++++-------- test/bash-preexec.bats | 70 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 12 deletions(-) diff --git a/bash-preexec.sh b/bash-preexec.sh index b998944..9e3f77e 100644 --- a/bash-preexec.sh +++ b/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 @@ -260,7 +277,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 @@ -269,7 +306,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="$?" @@ -278,14 +315,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() { diff --git a/test/bash-preexec.bats b/test/bash-preexec.bats index 152d5a6..83263f0 100644 --- a/test/bash-preexec.bats +++ b/test/bash-preexec.bats @@ -308,6 +308,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' From cbd3390fc75627f21849ea462e1173da4e199b07 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 22 Jan 2022 16:31:28 +0900 Subject: [PATCH 2/2] Add APIs for external precmd/preexec integrations * Rename public functions and variables as "bash_preexec_*" * Remove the compatibility variable name "__bp_install_string" * Add a note on the "trace" function attribute * Preserve the previous exit status and argument * Test the installation of convenience functions * Test "bash_preexec_uninstall" --- bash-preexec.sh | 70 ++++++++++++++++++++++++++++++------- test/bash-preexec.bats | 78 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 122 insertions(+), 26 deletions(-) diff --git a/bash-preexec.sh b/bash-preexec.sh index 9e3f77e..4977755 100644 --- a/bash-preexec.sh +++ b/bash-preexec.sh @@ -71,7 +71,11 @@ __bp_inside_precmd=0 __bp_inside_preexec=0 # Initial PROMPT_COMMAND string that is removed from PROMPT_COMMAND post __bp_install -__bp_install_string=$'__bp_trap_string="$(trap -p DEBUG)"\ntrap - DEBUG\n__bp_install' +bash_preexec_install_string=$'__bp_trap_string="$(trap -p DEBUG)"\ntrap - DEBUG\n__bp_install' + +# The command string that is registered to the DEBUG trap. +# shellcheck disable=SC2016 +bash_preexec_trapdebug_string='__bp_preexec_invoke_exec "$_"' # Fails if any of the given variables are readonly # Reference https://stackoverflow.com/a/4441178 @@ -157,7 +161,7 @@ __bp_precmd_invoke_cmd() { return fi local __bp_inside_precmd=1 - __bp_invoke_precmd_functions "$__bp_last_ret_value" "$__bp_last_argument_prev_command" + bash_preexec_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" } @@ -167,7 +171,7 @@ __bp_precmd_invoke_cmd() { # $_, 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() { +bash_preexec_invoke_precmd_functions() { local lastexit=$1 lastarg=$2 # Invoke every function defined in our function array. local precmd_function @@ -277,7 +281,7 @@ __bp_preexec_invoke_exec() { return fi - __bp_invoke_preexec_functions "${__bp_last_ret_value:-}" "$__bp_last_argument_prev_command" "$this_command" + bash_preexec_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 @@ -296,7 +300,7 @@ __bp_preexec_invoke_exec() { # (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() { +bash_preexec_invoke_preexec_functions() { local lastexit=$1 lastarg=$2 this_command=$3 local preexec_function local preexec_function_ret_value @@ -324,7 +328,8 @@ __bp_install() { return 1 fi - trap '__bp_preexec_invoke_exec "$_"' DEBUG + # shellcheck disable=SC2064 + trap "$bash_preexec_trapdebug_string" DEBUG # Preserve any prior DEBUG trap as a preexec function local prior_trap @@ -357,7 +362,7 @@ __bp_install() { # Remove setting our trap install string and sanitize the existing prompt command string existing_prompt_command="${PROMPT_COMMAND:-}" # Edge case of appending to PROMPT_COMMAND - existing_prompt_command="${existing_prompt_command//$__bp_install_string/:}" # no-op + existing_prompt_command="${existing_prompt_command//$bash_preexec_install_string/:}" # no-op existing_prompt_command="${existing_prompt_command//$'\n':$'\n'/$'\n'}" # remove known-token only existing_prompt_command="${existing_prompt_command//$'\n':;/$'\n'}" # remove known-token only __bp_sanitize_string existing_prompt_command "$existing_prompt_command" @@ -376,10 +381,13 @@ __bp_install() { PROMPT_COMMAND+=$'\n__bp_interactive_mode' fi - # Add two functions to our arrays for convenience - # of definition. - precmd_functions+=(precmd) - preexec_functions+=(preexec) + # Add two functions to our arrays for convenience of definition only when + # the functions have not yet added. + if [[ ! ${__bp_installed_convenience_functions-} ]]; then + __bp_installed_convenience_functions=1 + precmd_functions+=(precmd) + preexec_functions+=(preexec) + fi # Invoke our two functions manually that were added to $PROMPT_COMMAND __bp_precmd_invoke_cmd @@ -401,8 +409,46 @@ __bp_install_after_session_init() { PROMPT_COMMAND=${sanitized_prompt_command}$'\n' fi # shellcheck disable=SC2179 # PROMPT_COMMAND is not an array in bash <= 5.0 - PROMPT_COMMAND+=${__bp_install_string} + PROMPT_COMMAND+=${bash_preexec_install_string} +} + +# Remove hooks installed in the DEBUG trap and PROMPT_COMMAND. +bash_preexec_uninstall() { + # Remove __bp_install hook from PROMPT_COMMAND + # shellcheck disable=SC2178 # PROMPT_COMMAND is not an array in bash <= 5.0 + if [[ ${PROMPT_COMMAND-} == *"$bash_preexec_install_string"* ]]; then + PROMPT_COMMAND="${PROMPT_COMMAND//${bash_preexec_install_string}[;$'\n']}" # Edge case of appending to PROMPT_COMMAND + PROMPT_COMMAND="${PROMPT_COMMAND//$bash_preexec_install_string}" + fi + + # Remove precmd hook from PROMPT_COMMAND + local i prompt_command + for i in "${!PROMPT_COMMAND[@]}"; do + prompt_command=${PROMPT_COMMAND[i]} + case $prompt_command in + __bp_precmd_invoke_cmd | __bp_interactive_mode) + prompt_command= ;; + *) + prompt_command=${prompt_command/#$'__bp_precmd_invoke_cmd\n'/$'\n'} + prompt_command=${prompt_command%$'\n__bp_interactive_mode'} + prompt_command=${prompt_command#$'\n'} + esac + PROMPT_COMMAND[i]=$prompt_command + done + + # Remove preexec hook in the DEBUG trap + local q="'" Q="'\''" + if [[ $(trap -p DEBUG) == "trap -- '${bash_preexec_trapdebug_string//$q/$Q}' DEBUG" ]]; then + if [[ ${__bp_trap_string-} ]]; then + eval -- "$__bp_trap_string" + else + trap - DEBUG + fi + fi } +# Note: We need to add "trace" attribute to the function so that "trap - DEBUG" +# inside the function takes an effect. +declare -ft bash_preexec_uninstall # Run our install so long as we're not delaying it. if [[ -z "${__bp_delay_install:-}" ]]; then diff --git a/test/bash-preexec.bats b/test/bash-preexec.bats index 83263f0..a48cd6d 100644 --- a/test/bash-preexec.bats +++ b/test/bash-preexec.bats @@ -76,11 +76,11 @@ set_exit_code_and_run_precmd() { # Assert that before running, the command contains the install string, and # afterwards it does not - [[ "$PROMPT_COMMAND" == *"$__bp_install_string"* ]] || return 1 + [[ "$PROMPT_COMMAND" == *"$bash_preexec_install_string"* ]] || return 1 eval_PROMPT_COMMAND - [[ "$PROMPT_COMMAND" != *"$__bp_install_string"* ]] || return 1 + [[ "$PROMPT_COMMAND" != *"$bash_preexec_install_string"* ]] || return 1 } @test "__bp_install should preserve an existing DEBUG trap" { @@ -103,6 +103,56 @@ set_exit_code_and_run_precmd() { (( trap_count_snapshot < trap_invoked_count )) } +@test "__bp_install should register convenience functions \"preexec\" and \"precmd\" only once" { + precmd_functions=() + preexec_functions=() + __bp_install + bash_preexec_uninstall + __bp_install + + count=0 + for hook in "${precmd_functions[@]}"; do + if [[ "$hook" == precmd ]] ; then + count=$((count+1)) + fi + done + [ "$count" == 1 ] + + count=0 + for hook in "${preexec_functions[@]}"; do + if [[ "$hook" == preexec ]] ; then + count=$((count+1)) + fi + done + [ "$count" == 1 ] +} + +@test "bash_preexec_uninstall should remove the hooks in DEBUG and PROMPT_COMMAND" { + __bp_install + + q="'" Q="'\''" + [[ "$(join_PROMPT_COMMAND)" == *"__bp_precmd_invoke_cmd"* ]] || return 1 + [[ "$(join_PROMPT_COMMAND)" == *"__bp_interactive_mode"* ]] || return 1 + [ "$(trap -p DEBUG)" == "trap -- '${bash_preexec_trapdebug_string//$q/$Q}' DEBUG" ] + + bash_preexec_uninstall + + q="'" Q="'\''" + [[ "$(join_PROMPT_COMMAND)" != *"__bp_precmd_invoke_cmd"* ]] || return 1 + [[ "$(join_PROMPT_COMMAND)" != *"__bp_interactive_mode"* ]] || return 1 + [ "$(trap -p DEBUG)" != "trap -- '${bash_preexec_trapdebug_string//$q/$Q}' DEBUG" ] +} + +@test "bash_preexec_uninstall should remove the unprocessed __bp_install hook in PROMPT_COMMAND" { + __bp_install_after_session_init + + [[ "$PROMPT_COMMAND" == *"$bash_preexec_install_string"* ]] + + bash_preexec_uninstall + + [[ "$PROMPT_COMMAND" != *"$bash_preexec_install_string"* ]] +} + @test "__bp_sanitize_string should remove semicolons and trim space" { __bp_sanitize_string output " true1; "$'\n' @@ -308,12 +358,12 @@ set_exit_code_and_run_precmd() { [ $status -eq 1 ] } -@test "__bp_invoke_precmd_functions should be transparent for \$? and \$_" { +@test "bash_preexec_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 + bash_preexec_invoke_precmd_functions 111 'vxxJlwNx9VPJDA' || true [ "$test1_lastexit" == 111 ] [ "$test1_lastarg" == 'vxxJlwNx9VPJDA' ] @@ -321,29 +371,29 @@ set_exit_code_and_run_precmd() { [ "$test2_lastarg" == 'vxxJlwNx9VPJDA' ] } -@test "__bp_invoke_precmd_functions returns the last non-zero exit status" { +@test "bash_preexec_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=$? + bash_preexec_invoke_precmd_functions 1 'lastarg' || status=$? [ "$status" == 38 ] precmd_functions=(tester3) status=0 - __bp_invoke_precmd_functions 1 'lastarg' || status=$? + bash_preexec_invoke_precmd_functions 1 'lastarg' || status=$? [ "$status" == 0 ] } -@test "__bp_invoke_preexec_functions should be transparent for \$? and \$_" { +@test "bash_preexec_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 + bash_preexec_invoke_preexec_functions 87 'ehQrzHTHtE2E7Q' 'command' || true [ "$test1_lastexit" == 87 ] [ "$test1_lastarg" == 'ehQrzHTHtE2E7Q' ] @@ -351,28 +401,28 @@ set_exit_code_and_run_precmd() { [ "$test2_lastarg" == 'ehQrzHTHtE2E7Q' ] } -@test "__bp_invoke_preexec_functions returns the last non-zero exit status" { +@test "bash_preexec_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=$? + bash_preexec_invoke_preexec_functions 1 'lastarg' 'command' || status=$? [ "$status" == 112 ] preexec_functions=(tester3) status=0 - __bp_invoke_preexec_functions 1 'lastarg' 'command' || status=$? + bash_preexec_invoke_preexec_functions 1 'lastarg' 'command' || status=$? [ "$status" == 0 ] } -@test "__bp_invoke_preexec_functions should supply a current command in the first argument" { +@test "bash_preexec_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 + bash_preexec_invoke_preexec_functions 1 'lastarg' 'UEVkErELArSwjA' || true [ "$test1_bash_command" == 'UEVkErELArSwjA' ] [ "$test2_bash_command" == 'UEVkErELArSwjA' ]