diff --git a/README.md b/README.md index eae9f5f..27ea57b 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,8 @@ curl https://raw.githubusercontent.com/rcaloras/bash-preexec/master/bash-preexec echo '[[ -f ~/.bash-preexec.sh ]] && source ~/.bash-preexec.sh' >> ~/.bashrc ``` +NOTE: this script may change your `HISTCONTROL` value by removing `ignorespace` and/or replacing `ignoreboth` with `ignoredups`. See [`HISTCONTROL` interaction](#histcontrol-interaction) for details. + ## Usage Two functions **preexec** and **precmd** can now be defined and they'll be automatically invoked by bash-preexec if they exist. @@ -91,6 +93,10 @@ export __bp_enable_subshells="true" ``` This is disabled by default due to buggy situations related to to `functrace` and Bash's `DEBUG trap`. See [Issue #25](https://github.com/rcaloras/bash-preexec/issues/25) +## `HISTCONTROL` interaction + +In order to be able to provide the last command text to the `preexec` hook, this script will remove `ignorespace` and/or will replace `ignoreboth` with `ignoredups` in your `HISTCONTROL` variable. It will remember if `HISTCONTROL` has been modified and will remove the last command from the history "manually", after reading the last command from the history list. This may cause issues when you have scripts that rely on the literal value of `HISTCONTROL` or manipulate history in their own ways. + ## Tests You can run tests using [Bats](https://github.com/bats-core/bats-core). ```bash diff --git a/bash-preexec.sh b/bash-preexec.sh index c653b4d..81abd3c 100644 --- a/bash-preexec.sh +++ b/bash-preexec.sh @@ -51,6 +51,7 @@ __bp_imported="defined" __bp_last_ret_value="$?" BP_PIPESTATUS=("${PIPESTATUS[@]}") __bp_last_argument_prev_command="$_" +__bp_ignorespace= __bp_inside_precmd=0 __bp_inside_preexec=0 @@ -64,23 +65,33 @@ __bp_require_not_readonly() { local var for var; do if ! ( unset "$var" 2> /dev/null ); then - echo "bash-preexec requires write access to ${var}" >&2 + echo "${BASH_SOURCE##*/} requires write access to ${var}" >&2 return 1 fi done } -# 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. +# 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. We then remove commands that start with a space from the +# history "manually", if either "ignorespace" or "ignoreboth" was part of +# HISTCONTROL. __bp_adjust_histcontrol() { local histcontrol - histcontrol="${HISTCONTROL//ignorespace}" + if [[ "$HISTCONTROL" == *"ignorespace"* || "$HISTCONTROL" == *"ignoreboth"* ]]; then + __bp_ignorespace=yes + fi + histcontrol="${HISTCONTROL//ignorespace:}" + histcontrol="${histcontrol//:ignorespace}" + histcontrol="${histcontrol//ignorespace}" # Replace ignoreboth with ignoredups if [[ "$histcontrol" == *"ignoreboth"* ]]; then - histcontrol="ignoredups:${histcontrol//ignoreboth}" + histcontrol="${histcontrol//ignoreboth:}" + histcontrol="${histcontrol//:ignoreboth}" + histcontrol="${histcontrol//ignoreboth}" + histcontrol="ignoredups${histcontrol:+:}${histcontrol}" fi; - export HISTCONTROL="$histcontrol" + HISTCONTROL="$histcontrol" } # This variable describes whether we are currently in "interactive mode"; @@ -207,8 +218,7 @@ __bp_preexec_invoke_exec() { 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. + # We're doing something related to displaying the prompt. return else # If we're in a subshell, then the prompt won't be re-displayed to put @@ -229,16 +239,36 @@ __bp_preexec_invoke_exec() { fi local this_command - this_command=$( - export LC_ALL=C - HISTTIMEFORMAT= builtin history 1 | sed '1 s/^ *[0-9][0-9]*[* ] //' - ) + if [[ "$HISTCONTROL" == *"ignorespace"* || "$HISTCONTROL" == *"ignoreboth"* ]] + then + this_command="${BASH_COMMAND:-}" + local __bp_ignorespace= + else + this_command="$( HISTTIMEFORMAT= builtin history 1 | sed '1 s/^ *[0-9][0-9]*[* ] //' )" + fi # Sanity check to make sure we have something to invoke our function with. if [[ -z "$this_command" ]]; then return fi + # If we have removed "ignorespace" or "ignoreboth" from HISTCONTROL + # during setup, we need to remove commands that start with a space from + # the history ourselves. + + # With bash 5.0 or above, we could have just ran + # + # builtin history -d -1 + # + # Negative indices for `-d` are not supported before 5.0, so we need to + # do some computation ourselves. + if [[ -n "$__bp_ignorespace" && "$this_command" == " "* ]]; then + builtin history -d "$( + export LC_ALL=C + HISTTIMEFORMAT= history 1 | sed '1 s/^ *\([0-9][0-9]*\).*/\1/' + )" + fi + # Invoke every function defined in our function array. local preexec_function local preexec_function_ret_value @@ -246,7 +276,6 @@ __bp_preexec_invoke_exec() { for preexec_function in "${preexec_functions[@]:-}"; do # 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:-} # Quote our function invocation to prevent issues with IFS diff --git a/test/bash-preexec.bats b/test/bash-preexec.bats index db56753..c470a50 100644 --- a/test/bash-preexec.bats +++ b/test/bash-preexec.bats @@ -306,18 +306,17 @@ test_preexec_echo() { # Should remove ignorespace HISTCONTROL="ignorespace:ignoredups:*" __bp_adjust_histcontrol - [ "$HISTCONTROL" == ":ignoredups:*" ] + [ "$HISTCONTROL" == "ignoredups:*" ] # Should remove ignoreboth and replace it with ignoredups HISTCONTROL="ignoreboth" __bp_adjust_histcontrol - [ "$HISTCONTROL" == "ignoredups:" ] + [ "$HISTCONTROL" == "ignoredups" ] # Handle a few inputs HISTCONTROL="ignoreboth:ignorespace:some_thing_else" __bp_adjust_histcontrol - echo "$HISTCONTROL" - [ "$HISTCONTROL" == "ignoredups:::some_thing_else" ] + [ "$HISTCONTROL" == "ignoredups:some_thing_else" ] } @@ -340,6 +339,7 @@ test_preexec_echo() { run '__bp_preexec_invoke_exec' [ $status -eq 0 ] + echo "__bp_preexec_invoke_exec: output: '$output'" [ "$output" == " this command has whitespace " ] } @@ -362,3 +362,33 @@ a multiline string'" ] [ $status -eq 0 ] [ "$output" == '-n' ] } + +@test "HISTCONTROL is updated, but ignorespace functionality is honoured" { + preexec_functions+=(test_preexec_echo) + HISTCONTROL=ignorespace:ignoreboth + + __bp_adjust_histcontrol + + [[ "$HISTCONTROL" == "ignoredups" ]] + + __bp_interactive_mode + + command1="this command is in the history" + + history -s "$command1" + run '__bp_preexec_invoke_exec' + [[ $status == 0 ]] + [[ "$output" == "$command1" ]] + last_history=$(HISTTIMEFORMAT= history 1 | sed '1 s/^ *[0-9][0-9]* *//') + [[ "$last_history" == "$command1" ]] + + command2=" this should not be in the history" + + history -s "$command2" + # we need to extract command history in the subshell, as the parent shell + # history is actually not affected. + output=$(__bp_preexec_invoke_exec && \ + printf "last_history: %s\n" "$(HISTTIMEFORMAT= history 1 | sed '1 s/^ *[0-9][0-9]* *//')" ) + [[ $status == 0 ]] + [[ "$output" == "$command2"$'\n'"last_history: $command1" ]] +}