The remaining whitespaces are harmless and do not cause behavioral
differences in actual application. However, this causes differences
in the expected results of test cases in test/bash-preexec.bats. We
want to remove the whitespaces to normalize the expected results. In
particular, we treat PROMPT_COMMAND as an array based on the Bash
version, which causes the version depedenence of the expected results
with the current implementation.
* To make the expected results of test cases consistent, this patch
changes __bp_sanitize_string to remove the whitespaces.
* We also use [:blank:] instead of the variable sp=$' \t'.
We try to remove the existing hooks before the re-adjustment of
PROMPT_COMMAND, but it might not work when another framework saves the
original value of `PROMPT_COMMAND` in another variable and tries to
call it from inside their `PROMPT_COMMAND` hook. For example,
Starship does that [1]. In such a case, our hooks would be called
several times unexpectedly. To avoid this situation, we process our
hooks only when the hooks are called at the top level.
[1] 3d47468414/src/init/starship.bash (L97)
These are oversights of commit 21a26bf.
* Suppress SC2128 for array PROMPT_COMMAND
* Suppress SC2016 for the command string containing expansions like
'__bp_install "$_"'
The Copilot agent suggested to check readonly variables in a finer way
so that PS0 is not checked when PS0 is not used according to the
current version of Bash. However, this duplicates the complicated
test for the version requirement (( BASH_VERSINFO[0] > 5 ||
(BASH_VERSINFO[0] == 5 && BASH_VERSINFO[1] >= 3) )) for selecting the
preexec strategy. However, it is more manageable to store the
currently selected preexec strategy in a variable, and switch the
behavior based on the saved strategy value.
For this purpose, this patch adds a new global variable
"$__bp_hook_preexec_proc" that contains the function name to install
the hook. Then, we switch the check for readonly variables based on
the value of "$__bp_hook_preexec_proc". Now, the conditional checks
in Bats tests also use this variable to switch the tests.
* Do not prefix local varnames with underscores
* Make "__bp_invoke_pre{cmd,exec}_functions" return the last non-zero exit status
* Test "__bp_invoke_pre{cmd,exec}_functions"
This is the "official" (under the bats-core organization) GitHub Action
for setting up Bats in a workflow. This removes the need to manually
install the `bats` package.
Also, update to the latest actions/checkout version and use the latest
Ubuntu image.
* Remove dependency on sed(1) for history processing
We post-process `history 1`'s output to extract the current command.
This processing needs to strip the leading history number, an optional
`*` character indicating whether the entry was modified (or a space),
and then a space separating character.
We were previously using sed(1) for this, but we can implement an
equivalent transformation using bash's native parameter expansion
syntax.
* Improved the substitution pattern
The history number is printed as "%5d" so we need to handle all possible
columns.
declare -a lines=(
"10000 history 1"
" 1000 history 1"
" 1 history 1"
" 1000* history 1"
" 1000 history 1"
)
for line in "${lines[@]}"; do
echo "${line#*[[:digit:]][* ] }"
done
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
* Exclude the `bind -x` commands from interactive commands
* Check COMP_POINT instead of possibly empty COMP_LINE
The check for COMP_LINE can fail because it can be empty in the
completion function for the empty command line "complete -F _func -E".
This is an oversight of changing `trap DEBUG` to `trap - DEBUG` in
https://github.com/rcaloras/bash-preexec/pull/106
The oversight did not cause a test error because the test always
succeeds. The problem is that even if __bp_install is broken and
fails to remove `trap - DEBUG`, the current test case failed to detect
the failure and produce a false negative. This patch fixes it.
* Fix the test for the Bash version for shellcheck
* Add parenthesis and whitespace for clarity
---------
Co-authored-by: Michael Diamond <dimo414@gmail.com>
Some libraries do things like `PROMPT_COMMAND="$PROMPT_COMMAND ; _new_addition"`, that's a space-semicolon-space. It would be counter-productive to try to account for all possibilities, so just swap the install string for a no-op.
Clean up no-op, it it's all that's left
Co-authored-by: Ryan Caloras <rcaloras@gmail.com>
* Disable remaining shellcheck false warning
* Avoid passing empty argument to `return`
* Avoid word splitting of the exit status
* Follow the shellcheck suggestion
* add shellcheck # shellcheck shell=bash directive to allow linting
* BP_PIPESTATUS may be unused in function __bp_precmd_invoke_cmd, ignore
* line 174 - SC2086 (info): Double quote to prevent globbing and word splitting.
* line 249 false alarm 'SC1007 (warning): Remove space after = ', added ignore.
* line 294 SC2155 (warning): Declare and assign separately to avoid masking return values.
* line 295 SC2001 (style): See if you can use ${variable//search/replace}. Added ignore
* line 323 SC1087 (error): Use braces when expanding arrays. Disable warning
* fix indentation
* line 249 false alarm 'SC1007 (warning): empty space to '' removed shellcheck directive
Co-authored-by: Adam Price <git@blinvisible.com>
* Return original value from `__bp_precmd_invoke_cmd`
WIthout this change, `__bp_precmd_invoke_cmd` may not restore `$?` to its original value after being run.
Since `__bp_precmd_invoke_cmd` get installed as the first thing in `PROMPT_COMMAND`, this will cause problems if there is an existing `PROMPT_COMMAND` (which is perhaps not aware of `bash-preexec`) that wants to do something with the exit status of the command that was just run.
* Updating tests to allow preserving original exist status for other commands in PROMPT_COMMAND
Co-authored-by: Ryan Caloras <rcaloras@alumni.cmu.edu>
* Remove the underscore prefix of "__bp_imported"
This formally exposes it as part of the public API.
* Fix Bats test after renaming the variable
* Rename the variable upon reviewer request
* Fix inconsistency after renaming variables
Variables were reanmed in 7fbc07fb4e, but
it didn’t cover the test or ReadMe. This commit fixes that.
