Exclude the `bind -x` commands from interactive commands

[akinomyoga]

Issue: preexec is called for keybindings set up by bind -x

Consider the following setup:

# bashrc

source bash-preexec.sh
function preexec { echo "==== $FUNCNAME ($1) ===="; }
function precmd { echo "==== $FUNCNAME ===="; }

function something { return 0; }
bind -x '"\C-t": something'

When I start a session with the above rcfile (bashrc.exclude-bind-x) and input "echo 1" RET C-t "echo 2" RET C-d, the result becomes this:

[murase@chatoyancy 1 bash-preexec]$ bash --rcfile bashrc.exclude-bind-x
==== precmd ====
[murase@chatoyancy 0 bash-preexec]$ echo 1
==== preexec (echo 1) ====
1
==== precmd ====
==== preexec (echo 1) ====
[murase@chatoyancy 0 bash-preexec]$ echo 2
2
==== precmd ====
[murase@chatoyancy 0 bash-preexec]$
exit

The second preexec is supposed to be called against the second command echo 2, but it's instead called for the command of the keybinding something, which is not an expected behavior.

This problem was identified based on the report at https://github.com/atuinsh/atuin/issues/1003#issuecomment-1947905213.

Solution

We can take the same solution as completion functions. For the completion functions, we are using COMP_LINE to test whether we are currently running the completion function. For the bind -x commands, we can test READLINE_POINT to test whether we are running a command for keybindings.

This solution can be broken when the user sets the shell variable READLINE_POINT, but it's not a problem specific to this PR. The existing code for COMP_LINE has the same problem when the user sets the variable. In reality, there shouldn't be many cases where the user sets READLINE_POINT outside the keybinding functions. Rather there are more cases where the users set up keybindings through bind -x.

Here's the behavior after this fix:

[murase@chatoyancy 1 bash-preexec]$ bash --rcfile bashrc.exclude-bind-x
==== precmd ====
[murase@chatoyancy 0 bash-preexec]$ echo 1
==== preexec (echo 1) ====
1
==== precmd ====
[murase@chatoyancy 0 bash-preexec]$ echo 2
==== preexec (echo 2) ====
2
==== precmd ====
[murase@chatoyancy 0 bash-preexec]$
exit

[akinomyoga]

I pushed another commit, where I suggest changing COMP_LINE to COMP_POINT. The variable COMP_LINE can be empty on the empty command line. For example, when a completion is attempted on the empty command line with the following setting, preexec can be unexpectedly called on pressing TAB.

$ complete -E -F _test1
$ _test1() { declare -p "${!COMP_@}"; }

[akinomyoga]

@dimo414 Could you review this PR? This fix is a very small one, so it shouldn't be difficult to review. Or is there anything missing?

[akinomyoga]

I thought about if it is possible to add a test, but we need an interactive session to directly test it. Also, the test for the existing COMP_LINE doesn't seem to exist. So I haven't included a test for this PR for now. Maybe we can set up a mock test. Or maybe we can use expect (I've never used it though), but this adds extra dependencies for the test. What do you think?

[rcaloras]

@akinomyoga thanks for the PR and fix as always. 👍

Sorry for delays as I've recently had extraneous family responsibilities. Recognize there's a few open issues/PRs the community is trying to push through. I'm interested in trying to get more folks approval access to support.

[akinomyoga]

@rcaloras Thank you for taking your time!

I'm interested in trying to get more folks approval access to support.

Maybe we can write something on README.md or CONTRIBUTING.md?