From 77a5515efbda84fa3af1eaf9edccd0a028bd4829 Mon Sep 17 00:00:00 2001 From: Ilya Bobyr Date: Tue, 16 Jul 2019 01:07:51 -0700 Subject: [PATCH] Honor HISTCONTROL ignorespace and ignoreboth After 3458480385f81124a9fd9a38ff384e43dd815b1c Remove ignorespace from $HISTCONTROL and after 7e55ac15fee86cdf8e08e966d8b8c9dd2bf7ad03 Follow up commit for issue #6 -Replace ignoreboth with simpley ignoredups this script would remove 'ignorespace' and would replace 'ignoreboth' with 'ignoredups'. This effectively disables the functionality of not adding space prefixed commands into history. It used to happen siliently and could be quite confusing to users who use this feature. This script relies on the command to be in the history, but we can mostly fix the issue by "manual" removing a whitespace prefixed command from the history after reading it from there. --- README.md | 6 ++++++ bash-preexec.sh | 38 +++++++++++++++++++++++++++++++++----- test/bash-preexec.bats | 38 ++++++++++++++++++++++++++++++++++---- 3 files changed, 73 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 3e88c84..ffb6fca 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 4380683..832c3b9 100644 --- a/bash-preexec.sh +++ b/bash-preexec.sh @@ -46,6 +46,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 @@ -62,15 +63,25 @@ __bp_require_not_readonly() { 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" } @@ -213,6 +224,23 @@ __bp_preexec_invoke_exec() { 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 + # If none of the previous checks have returned out of this function, then # the command is in fact interactive and we should invoke the user's # preexec functions. diff --git a/test/bash-preexec.bats b/test/bash-preexec.bats index 42fd43e..4c0d1b5 100644 --- a/test/bash-preexec.bats +++ b/test/bash-preexec.bats @@ -236,18 +236,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" ] } @@ -270,6 +269,7 @@ test_preexec_echo() { run '__bp_preexec_invoke_exec' [ $status -eq 0 ] + echo "__bp_preexec_invoke_exec: output: '$output'" [ "$output" == " this command has whitespace " ] } @@ -292,3 +292,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" ]] +}