Ensure a pre-existing DEBUG trap is preserved as a preexec function.

bash-preexec.sh

@@ -26,13 +26,13 @@
 #
 #       preexec_functions+=(my_preexec_function)
 #
-#  3. If you have anything that's using the Debug Trap, change it to use
-#     preexec. (Optional) change anything using PROMPT_COMMAND to now use
-#     precmd instead.
+#  3. Consider changing anything using the DEBUG trap or PROMPT_COMMAND
+#     to use preexec and precmd instead. Preexisting usages will be
+#     preserved, but doing so manually may be less surprising.
 #
-#  Note: This module requires two bash features which you must not otherwise be
-#  using: the "DEBUG" trap, and the "PROMPT_COMMAND" variable. prexec_and_precmd_install
-#  will override these and if you override one or the other this will most likely break.
+#  Note: This module requires two Bash features which you must not otherwise be
+#  using: the "DEBUG" trap, and the "PROMPT_COMMAND" variable. If you override
+#  either of these after bash-preexec has been installed it will most likely break.
 
 # Avoid duplicate inclusion
 if [[ "$__bp_imported" == "defined" ]]; then
@@ -45,10 +45,6 @@ __bp_imported="defined"
 __bp_last_ret_value="$?"
 __bp_last_argument_prev_command="$_"
 
-# Command to set our preexec trap. It's invoked once via
-# PROMPT_COMMAND and then removed.
-__bp_trap_install_string="trap '__bp_preexec_invoke_exec \"\$_\"' DEBUG;"
-
 # Remove ignorespace and or replace ignoreboth from HISTCONTROL
 # so we can accurately invoke preexec with a command from our
 # history even if it starts with a space.
@@ -137,8 +133,6 @@ __bp_in_prompt_command() {
 # environment to attempt to detect if the current command is being invoked
 # interactively, and invoke 'preexec' if so.
 __bp_preexec_invoke_exec() {
-
-
     # Save the contents of $_ so that it can be restored later on.
     # https://stackoverflow.com/questions/40944532/bash-preserve-in-a-debug-trap#40944702
     __bp_last_argument_prev_command="$1"
@@ -216,41 +210,24 @@ __bp_preexec_invoke_exec() {
     __bp_set_ret_value "$preexec_ret_value" "$__bp_last_argument_prev_command"
 }
 
-# Returns PROMPT_COMMAND with a semicolon appended
-# if it doesn't already have one.
-__bp_prompt_command_with_semi_colon() {
-
-    # Trim our existing PROMPT_COMMAND
-    local trimmed
-    trimmed=$(__bp_trim_whitespace "$PROMPT_COMMAND")
-
-    # Take our existing prompt command and append a semicolon to it
-    # if it doesn't already have one.
-    local existing_prompt_command
-    if [[ -n "$trimmed" ]]; then
-        existing_prompt_command=${trimmed%${trimmed##*[![:space:]]}}
-        existing_prompt_command=${existing_prompt_command%;}
-        existing_prompt_command=${existing_prompt_command/%/;}
-    else
-        existing_prompt_command=""
-    fi
-
-    echo -n "$existing_prompt_command"
-}
-
 __bp_install() {
-
-    # Remove setting our trap from PROMPT_COMMAND
-    PROMPT_COMMAND="${PROMPT_COMMAND//$__bp_trap_install_string}"
-
-    # Remove this function from our PROMPT_COMMAND
-    PROMPT_COMMAND="${PROMPT_COMMAND//__bp_install;}"
-
     # Exit if we already have this installed.
     if [[ "$PROMPT_COMMAND" == *"__bp_precmd_invoke_cmd"* ]]; then
         return 1;
     fi
 
+    trap '__bp_preexec_invoke_exec "$_"' DEBUG
+
+    # Preserve any prior DEBUG trap as a preexec function
+    local prior_trap=$(sed "s/[^']*'\(.*\)'[^']*/\1/" <<<"$__bp_trap_string")
+    unset __bp_trap_string
+    if [[ -n "$prior_trap" ]]; then
+        eval '__bp_original_debug_trap() {
+          '"$prior_trap"'
+        }'
+        preexec_functions+=(__bp_original_debug_trap)
+    fi
+
     # Adjust our HISTCONTROL Variable if needed.
     __bp_adjust_histcontrol
 
@@ -266,24 +243,17 @@ __bp_install() {
         shopt -s extdebug > /dev/null 2>&1
     fi;
 
-
-    local existing_prompt_command
-    existing_prompt_command=$(__bp_prompt_command_with_semi_colon)
-
     # Install our hooks in PROMPT_COMMAND to allow our trap to know when we've
     # actually entered something.
-    PROMPT_COMMAND="__bp_precmd_invoke_cmd; ${existing_prompt_command} __bp_interactive_mode"
-    eval "$__bp_trap_install_string"
+    PROMPT_COMMAND="__bp_precmd_invoke_cmd; __bp_interactive_mode"
 
     # Add two functions to our arrays for convenience
     # of definition.
     precmd_functions+=(precmd)
     preexec_functions+=(preexec)
 
-    # Since this is in PROMPT_COMMAND, invoke any precmd functions we have defined.
-    __bp_precmd_invoke_cmd
-    # Put us in interactive mode for our first command.
-    __bp_interactive_mode
+    # Since this function is invoked via PROMPT_COMMAND, re-execute PC now that it's properly set
+    eval "$PROMPT_COMMAND"
 }
 
 # Sets our trap and __bp_install as part of our PROMPT_COMMAND to install
@@ -298,12 +268,20 @@ __bp_install_after_session_init() {
         return 1;
     fi
 
-    local existing_prompt_command
-    existing_prompt_command=$(__bp_prompt_command_with_semi_colon)
+    # If there's an existing PROMPT_COMMAND capture it and convert it into a function
+    # So it is preserved and invoked during precmd.
+    if [[ -n "$PROMPT_COMMAND" ]]; then
+      eval '__bp_original_prompt_command() {
+        '"$PROMPT_COMMAND"'
+      }'
+      precmd_functions+=(__bp_original_prompt_command)
+    fi
 
-    # Add our installation to be done last via our PROMPT_COMMAND. These are
-    # removed by __bp_install when it's invoked so it only runs once.
-    PROMPT_COMMAND="${existing_prompt_command} $__bp_trap_install_string __bp_install;"
+    # Installation is finalized in PROMPT_COMMAND, which allows us to override the DEBUG
+    # trap. __bp_install sets PROMPT_COMMAND to its final value, so these are only
+    # invoked once.
+    # It's necessary to clear any existing DEBUG trap in order to set it from the install function.
+    PROMPT_COMMAND='__bp_trap_string=$(trap -p DEBUG); trap DEBUG; __bp_install'
 }
 
 # Run our install so long as we're not delaying it.

test/bash-preexec.bats

@@ -1,10 +1,17 @@
 #!/usr/bin/env bats
 
 setup() {
+  PROMPT_COMMAND=''        # in case the invoking shell has set this
+  history -s fake command  # preexec requires there be some history
   __bp_delay_install="true"
   source "${BATS_TEST_DIRNAME}/../bash-preexec.sh"
 }
 
+bp_install() {
+  __bp_install_after_session_init
+  eval "$PROMPT_COMMAND"
+}
+
 test_echo() {
   echo "test echo"
 }
@@ -21,60 +28,54 @@ test_preexec_echo() {
 }
 
 @test "__bp_install should exit if it's already installed" {
-  PROMPT_COMMAND="some_other_function; __bp_precmd_invoke_cmd;"
+  bp_install
+
   run '__bp_install'
   [[ $status == 1 ]]
   [[ -z "$output" ]]
 }
 
-@test "__bp_install should remove trap and itself from PROMPT_COMMAND" {
-  trap_string="trap '__bp_preexec_invoke_exec \"\$_\"' DEBUG;"
-  PROMPT_COMMAND="some_other_function; $trap_string __bp_install;"
+@test "__bp_install should remove trap logic and itself from PROMPT_COMMAND" {
+  __bp_install_after_session_init
 
-  [[ $PROMPT_COMMAND  == *"$trap_string"* ]]
-  [[ $PROMPT_COMMAND  = *"__bp_install;"* ]]
+  [[ "$PROMPT_COMMAND" == *"trap DEBUG"* ]]
+  [[ "$PROMPT_COMMAND" == *"__bp_install"* ]]
 
-  __bp_install
+  eval "$PROMPT_COMMAND"
 
-  [[ $PROMPT_COMMAND  != *"$trap_string"* ]]
-  [[ $PROMPT_COMMAND  != *"__bp_install;"* ]]
+  [[ "$PROMPT_COMMAND" != *"trap DEBUG"* ]]
+  [[ "$PROMPT_COMMAND" != *"__bp_install"* ]]
 }
 
-@test "PROMPT_COMMAND=\"\$PROMPT_COMMAND; foo\" should work" {
-    __bp_install
-
-    PROMPT_COMMAND="$PROMPT_COMMAND; true"
-    eval "$PROMPT_COMMAND"
-}
+@test "__bp_install should preserve an existing DEBUG trap" {
+  trap_invoked_count=0
+  foo() { (( trap_invoked_count += 1 )); }
 
-@test "__bp_prompt_command_with_semi_colon should handle different PROMPT_COMMANDS" {
-    # PROMPT_COMMAND of spaces
-    PROMPT_COMMAND=" "
+  # note setting this causes BATS to mis-report the failure line when this test fails
+  trap foo DEBUG
+  [[ "$(trap -p DEBUG | cut -d' ' -f3)" == "'foo'" ]]
 
-    run '__bp_prompt_command_with_semi_colon'
-    [[ -z "$output" ]]
+  bp_install
+  trap_count_snapshot=$trap_invoked_count
 
-    # PROMPT_COMMAND of one command
-    PROMPT_COMMAND="echo 'yo'"
+  [[ "$(trap -p DEBUG | cut -d' ' -f3)" == "'__bp_preexec_invoke_exec" ]]
+  [[ "${preexec_functions[*]}" == *"__bp_original_debug_trap"* ]]
 
-    run '__bp_prompt_command_with_semi_colon'
-    [[ "$output" == "echo 'yo';" ]]
+  __bp_interactive_mode # triggers the DEBUG trap
 
-    # No PROMPT_COMMAND
-    unset PROMPT_COMMAND
-    run '__bp_prompt_command_with_semi_colon'
-    [[ -z "$output" ]]
+  # ensure the trap count is still being incremented after the trap's been overwritten
+  (( trap_count_snapshot < trap_invoked_count ))
+}
 
-    # PROMPT_COMMAND of two commands and trimmed
-    PROMPT_COMMAND="echo 'yo'; ls    "
+@test "PROMPT_COMMAND=\"\$PROMPT_COMMAND; foo\" should work" {
+    bp_install
 
-    run '__bp_prompt_command_with_semi_colon'
-    [[ "$output" == "echo 'yo'; ls;" ]]
+    PROMPT_COMMAND="$PROMPT_COMMAND; true"
+    eval "$PROMPT_COMMAND"
 }
 
 @test "No functions defined for preexec should simply return" {
-    __bp_preexec_interactive_mode="on"
-    history -s "fake command"
+    __bp_interactive_mode
 
     run '__bp_preexec_invoke_exec' 'true'
     [[ $status == 0 ]]
@@ -91,15 +92,18 @@ test_preexec_echo() {
 @test "precmd should set \$? to be the previous exit code" {
     echo_exit_code() {
       echo "$?"
-      return 0
     }
-    precmd_functions+=(echo_exit_code)
-
-    __bp_set_ret_value() {
-      return 251
+    return_exit_code() {
+      return $1
+    }
+    # Helper function is necessary because Bats' run doesn't preserve $?
+    set_exit_code_and_run_precmd() {
+      return_exit_code 251
+      __bp_precmd_invoke_cmd
     }
 
-    run '__bp_precmd_invoke_cmd'
+    precmd_functions+=(echo_exit_code)
+    run 'set_exit_code_and_run_precmd'
     [[ $status == 0 ]]
     [[ "$output" == "251" ]]
 }
@@ -110,17 +114,20 @@ test_preexec_echo() {
     }
     precmd_functions+=(echo_last_arg)
 
-    __bp_last_argument_prev_command="last-arg"
-
+    bats_trap=$(trap -p DEBUG)
+    trap DEBUG # remove the Bats stack-trace trap so $_ doesn't get overwritten
+    : "last-arg"
+    __bp_preexec_invoke_exec "$_"
+    eval "$bats_trap" # Restore trap
     run '__bp_precmd_invoke_cmd'
     [[ $status == 0 ]]
     [[ "$output" == "last-arg" ]]
 }
 
 @test "preexec should execute a function with the last command in our history" {
     preexec_functions+=(test_preexec_echo)
-    __bp_preexec_interactive_mode="on"
-    git_command="git commit -a -m 'commiting some stuff'"
+    __bp_interactive_mode
+    git_command="git commit -a -m 'committing some stuff'"
     history -s $git_command
 
     run '__bp_preexec_invoke_exec'
@@ -133,11 +140,11 @@ test_preexec_echo() {
     fun_2() { echo "$1 two"; }
     preexec_functions+=(fun_1)
     preexec_functions+=(fun_2)
-    __bp_preexec_interactive_mode="on"
-    history -s "fake command"
+    __bp_interactive_mode
 
     run '__bp_preexec_invoke_exec'
     [[ $status == 0 ]]
+    [[ "${#lines[@]}" == 2 ]]
     [[ "${lines[0]}" == "fake command one" ]]
     [[ "${lines[1]}" == "fake command two" ]]
 }
@@ -150,6 +157,7 @@ test_preexec_echo() {
 
     run '__bp_precmd_invoke_cmd'
     [[ $status == 0 ]]
+    [[ "${#lines[@]}" == 2 ]]
     [[ "${lines[0]}" == "one" ]]
     [[ "${lines[1]}" == "two" ]]
 }
@@ -160,8 +168,7 @@ test_preexec_echo() {
     }
     preexec_functions+=(return_nonzero)
 
-    __bp_preexec_interactive_mode="on"
-    history -s "fake command"
+    __bp_interactive_mode
 
     run '__bp_preexec_invoke_exec'
     [[ $status == 1 ]]
@@ -209,8 +216,8 @@ test_preexec_echo() {
 
 @test "preexec should respect HISTTIMEFORMAT" {
     preexec_functions+=(test_preexec_echo)
-    __bp_preexec_interactive_mode="on"
-    git_command="git commit -a -m 'commiting some stuff'"
+    __bp_interactive_mode
+    git_command="git commit -a -m 'committing some stuff'"
     HISTTIMEFORMAT='%F %T '
     history -s $git_command
 

test/include-test.bats

@@ -6,7 +6,7 @@
   [[ -z $(type -t __bp_preexec_and_precmd_install) ]]
 }
 
-@test "should import of not defined" {
+@test "should import if not defined" {
   unset __bp_imported
   source "${BATS_TEST_DIRNAME}/../bash-preexec.sh"
   [[ -n $(type -t __bp_install) ]]