Honor HISTCONTROL ignorespace and ignoreboth

After 3458480

    Remove ignorespace from $HISTCONTROL

and after 7e55ac1

    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.

Author: ilya-bobyr

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,12 @@ 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.
+
+Unfortunately, this only works with Bash 5.0 and above.  In older version of Bash, `ignorespace` and `ignoreboth` will be just silently ignored.
+
 ## Tests
 You can run tests using [Bats](https://github.com/bats-core/bats-core).
 ```bash

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,18 @@ __bp_preexec_invoke_exec() {
         return
     fi
 
+    # If we have remove "ignorespace" or "ignoreboth" from HISTCONTROL during
+    # setup, we need to remove commands that start with a space from the
+    # history ourselves.
+
+    # Unfortunately, we need bash 5.0 or above, as "history -d" does not
+    # support negative indices in earlier versions and HISTCMD seems to be
+    # unreliable.  At least in the unit tests HISTCMD is just set to 1, after
+    # any number of "history -s" calls.
+    if [[ "${BASH_VERSINFO:-0}" -ge 5 && -n "$__bp_ignorespace" && "$this_command" == " "* ]]; then
+      builtin history -d -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.

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" ]
 
 }
 
@@ -292,3 +291,37 @@ a multiline string'" ]
     [ $status -eq 0 ]
     [ "$output" == '-n' ]
 }
+
+@test "HISTCONTROL is updated, but ignorespace functionality is honoured" {
+    if [ "${BASH_VERSINFO:-0}" -lt 5 ]; then
+        skip "Proper history manipulation is supported only since Bash 5.0"
+    fi
+
+    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" ]]
+}