From df2d0b33cc46160f834ecc13acfdcdbe34b536ad Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Thu, 16 Jan 2025 08:30:27 -0500 Subject: [PATCH] bash: improve prior_trap processing We use `trap` to bootstrap our installation function (__bp_install). We remove our code upon first execution but need to restore any preexisting trap calls. We previously used `sed` to process the trap string, but that had two downsides: 1. `sed` is an external command dependency. It needs to exist on the system, and we need to invoke it in a subshell (which has some runtime cost). 2. The regular expression pattern was imperfect and didn't handle trickier cases like `'` characters in the trap string: $ (trap "echo 'hello'" DEBUG; trap -p DEBUG) hello trap -- 'echo '\''hello'\''' DEBUG This change removes the dependency on `sed` by locally evaluating the trap string and extracting any prior trap. This works reliably because we control the format our trap string, which looks like this (with newlines expanded): __bp_trap_string="$(trap -p DEBUG)" trap - DEBUG __bp_install --- src/shell-integration/bash/bash-preexec.sh | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/shell-integration/bash/bash-preexec.sh b/src/shell-integration/bash/bash-preexec.sh index 14a677888..9d3357387 100644 --- a/src/shell-integration/bash/bash-preexec.sh +++ b/src/shell-integration/bash/bash-preexec.sh @@ -297,10 +297,8 @@ __bp_install() { trap '__bp_preexec_invoke_exec "$_"' DEBUG # Preserve any prior DEBUG trap as a preexec function - local prior_trap - # we can't easily do this with variable expansion. Leaving as sed command. - # shellcheck disable=SC2001 - prior_trap=$(sed "s/[^']*'\(.*\)'[^']*/\1/" <<<"${__bp_trap_string:-}") + eval "local trap_argv=(${__bp_trap_string:-})" + local prior_trap=${trap_argv[2]:-} unset __bp_trap_string if [[ -n "$prior_trap" ]]; then eval '__bp_original_debug_trap() {