From 58de74b5c8f99105b416b5537669268665d2e0b1 Mon Sep 17 00:00:00 2001 From: "Guillaume Destuynder (:kang)" <956866+gdestuynder@users.noreply.github.com> Date: Mon, 11 May 2020 15:19:13 -0700 Subject: [PATCH 1/3] Fix condition where pre-exec hooks would not trigger When running a pre-exec hook/function for `ls ; ls` for example, pre-exec would only occur on the first `ls` command because the 2nd iteration is considered to be "interactive while prompt is being displayed". This is an issue if your hook sets a return value in order to stop the command from executing. Ex of condition that this would fix: ``` $ preexec() { return 1; } # this should cause all commands to not run/fail $ ls -l $ # This failed properly $ ls -l && ls -l $ # The 2nd ls -l triggered, instead of calling the pre-exec hook ``` --- bash-preexec.sh | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/bash-preexec.sh b/bash-preexec.sh index ce09a4f..f91a283 100644 --- a/bash-preexec.sh +++ b/bash-preexec.sh @@ -133,6 +133,13 @@ __bp_set_ret_value() { return ${1:-} } +# Sets the last command we ran. This is used to check if we're still executing +# the same command at the next run of preexec (e.g. for `ls ; ls` we will +# trigger through the DEBUG trap twice. +__bp_set_last_command() { + __bp_last_command="$1" + } + __bp_in_prompt_command() { local prompt_command_array @@ -180,9 +187,18 @@ __bp_preexec_invoke_exec() { # an interactively issued command. return fi - if [[ -z "${__bp_preexec_interactive_mode:-}" ]]; then - # We're doing something related to displaying the prompt. Let the - # prompt set the title instead of me. + + local this_command + this_command=$( + export LC_ALL=C + HISTTIMEFORMAT= builtin history 1 | sed '1 s/^ *[0-9][0-9]*[* ] //' + ) + + if [[ -z "${__bp_preexec_interactive_mode:-}" && \ + "$this_command" != "$__bp_last_command" ]]; then + # We're doing something related to displaying the prompt and this + # isn't a follow-up to our last command, let the prompt set the + # title instead of me. return else # If we're in a subshell, then the prompt won't be re-displayed to put @@ -202,12 +218,6 @@ __bp_preexec_invoke_exec() { return fi - local this_command - this_command=$( - export LC_ALL=C - HISTTIMEFORMAT= builtin history 1 | sed '1 s/^ *[0-9][0-9]*[* ] //' - ) - # Sanity check to make sure we have something to invoke our function with. if [[ -z "$this_command" ]]; then return @@ -226,6 +236,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_last_command "$this_command" __bp_set_ret_value ${__bp_last_ret_value:-} # Quote our function invocation to prevent issues with IFS "$preexec_function" "$this_command" @@ -242,6 +253,7 @@ __bp_preexec_invoke_exec() { # 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_last_command "this_command" __bp_set_ret_value "$preexec_ret_value" "$__bp_last_argument_prev_command" } From e7decb4210d95bfd17bacdcd4e613b5b2b306bf4 Mon Sep 17 00:00:00 2001 From: Guillaume Destuynder <956866+gdestuynder@users.noreply.github.com> Date: Tue, 12 May 2020 15:31:28 -0700 Subject: [PATCH 2/3] Fix typos --- bash-preexec.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bash-preexec.sh b/bash-preexec.sh index f91a283..486ba09 100644 --- a/bash-preexec.sh +++ b/bash-preexec.sh @@ -138,7 +138,7 @@ __bp_set_ret_value() { # trigger through the DEBUG trap twice. __bp_set_last_command() { __bp_last_command="$1" - } +} __bp_in_prompt_command() { @@ -253,7 +253,7 @@ __bp_preexec_invoke_exec() { # 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_last_command "this_command" + __bp_set_last_command "$this_command" __bp_set_ret_value "$preexec_ret_value" "$__bp_last_argument_prev_command" } From e8eba9795ffefc9979871a49588fb9444c55bd52 Mon Sep 17 00:00:00 2001 From: Guillaume Destuynder <956866+gdestuynder@users.noreply.github.com> Date: Tue, 12 May 2020 15:31:40 -0700 Subject: [PATCH 3/3] Add tests for preexec "last_command" --- test/bash-preexec.bats | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/bash-preexec.bats b/test/bash-preexec.bats index 66dbe23..ed0cc6a 100644 --- a/test/bash-preexec.bats +++ b/test/bash-preexec.bats @@ -21,6 +21,11 @@ test_preexec_echo() { printf "%s\n" "$1" } +# This function is used when you need a non-zero return, as zero is the default +test_preexec_ret_error() { + return 1 +} + @test "__bp_install_after_session_init should exit with 1 if we're not using bash" { unset BASH_VERSION run '__bp_install_after_session_init' @@ -292,3 +297,21 @@ a multiline string'" ] [ $status -eq 0 ] [ "$output" == '-n' ] } + +@test "preexec should trigger when several processes are executed, from the same interactive CLI command" { + preexec_functions+=(test_preexec_ret_error) + history -s -- 'ls;ls' + __bp_interactive_mode + run '__bp_preexec_invoke_exec' + # __bp_interactive_mode is now set to empty string, but history still matches + # so we should still be running the preexec hook when the second `ls` runs. + run '__bp_preexec_invoke_exec' + [ $status -eq 1 ] + + # We should now exit without running the preexec hook + # which results in the return defaulting to 0. + __bp_preexec_interactive_mode="" + __bp_last_command="" + run '__bp_preexec_invoke_exec' + [ $status -eq 0 ] +}