Don't clobber $_

[HaleTom]

$_ should give the last argument of the last command.

Currently it gives __bp_preexec_invoke_exec:

$ exec bash
$ . ./bash-preexec.sh
$ preexec() { echo "just typed $1"; }
$ echo $_
just typed echo $_
__bp_preexec_invoke_exec
$

Fix this to produce the expected behaviour:

exec bash
$ . ./bash-preexec.sh
$ preexec() { echo "just typed $1"; }
$ echo $_
just typed echo $_
./bash-preexec.sh

Note that bash-bats currently suffers from the same bug (it's a feature of bash), so the newly added test fails. I have manually tested it and it passes.

[rcaloras]

@HaleTom Thanks for the PR! I'm traveling at the moment, but I'll give this a look in a few days and would love to merge this in.

[rcaloras]

@HaleTom Just updated the PR. I think all it needs is to have $_ passed additionally in the trap function. That effectively forwards the value so it should be available afterwards. Setting it in __bp_preexec_invoke_exec via : "$old_" is unnecessary I believe. Tested and seemed fine for me locally.

Give it a try. If it looks good I'll merge 👍 Thanks for your help!

[HaleTom]

Not for me:

@ravi@genesis:~/repo/bash-preexec(fix-dollar_underscore)$ source bash-preexec.sh
@ravi@genesis:~/repo/bash-preexec(fix-dollar_underscore)$ echo $_
__bp_preexec_invoke_exec
@ravi@genesis:~/repo/bash-preexec(fix-dollar_underscore)$ echo 1 2 3
1 2 3
@ravi@genesis:~/repo/bash-preexec(fix-dollar_underscore)$ echo $_
__bp_preexec_invoke_exec
@ravi@genesis:~/repo/bash-preexec(fix-dollar_underscore)$ 

GNU bash, version 4.4.5(1)-release (x86_64-unknown-linux-gnu)

[rcaloras]

Hmm is that in the same session as your previous version of bash-preexec? bash-preexec won't override a previous version that's already included in the current bash session by checking __bp_imported. That could be causing the issue.

I just tried testing this PR on a slightly older version of bash 4.4 and I can't recreate :/

elementz@Kashmir:~/git/HaleTom/bash-preexec (fix-dollar_underscore)$ source bash-preexec.sh 
elementz@Kashmir:~/git/HaleTom/bash-preexec (fix-dollar_underscore)$ echo $BASH_VERSION
4.4.0(1)-release
elementz@Kashmir:~/git/HaleTom/bash-preexec (fix-dollar_underscore)$ echo $_
4.4.0(1)-release
elementz@Kashmir:~/git/HaleTom/bash-preexec (fix-dollar_underscore)$ : 1 2 3
elementz@Kashmir:~/git/HaleTom/bash-preexec (fix-dollar_underscore)$ echo $_
3
elementz@Kashmir:~/git/HaleTom/bash-preexec (fix-dollar_underscore)$ preexec() { echo "Hello $1"; }
elementz@Kashmir:~/git/HaleTom/bash-preexec (fix-dollar_underscore)$ echo $_                       
Hello echo $_
3
elementz@Kashmir:~/git/HaleTom/bash-preexec (fix-dollar_underscore)$ 

Let me see if I can get a version of 4.4.5 to try as well.

[HaleTom]

I just double checked - no previous sourcing of bash-preexec.sh but still the same behaviour. chruby was also disabled.

I have:
PROMPT_COMMAND=__set_bash_prompt... I'm assuming that this shouldn't affect anything.

[rcaloras]

Hmm okay, I'll add back your preservation of $_ inside _bp_preexec_invoke_exec. Will push in a min.

[HaleTom]

What about the test... another PR when I get around to fixing bats?

[rcaloras]

Pushed that latest, @HaleTom give it a try locally.

The previous test case added was not correct. I removed it in my first commit. I don't see an easy way to test this since it depends on the internal workings of bash to update $_ and the debug trap. Bats is more for unit tests on individual functions. If this patchset works for you, we can give it some more thought regarding a potential test case.

[rcaloras]

Bump. @HaleTom any chance to review the latest?

[HaleTom]

I haven't forgotten about this - I'm just rebuilding a server and doing the usual Festivus activities.

It will likely be two weeks as I'm also moving through 2 countries in that time.

Happy New Year!

[rcaloras]

@HaleTom no worries. Please give it a try when you get a moment.

If anyone else in the community has a sec, please give this a run to see if $_ is being preserved. It's working locally for me, but I want to test across a few other systems and bash versions.

[rcaloras]

@HaleTom Bump! Got a sec to test this PR?

[rcaloras]

@HaleTom I'm planning on merging this and cutting 3.2. Please check it against your setup when you're available.

[rcaloras]

👍 🍔 🍟

[rcaloras]

@HaleTom feel free to reopen as needed. Thanks for your contribution and opening this.