dylanaraps / pure-bash-bible

📖 A collection of pure bash alternatives to external processes.
MIT License
36.53k stars 3.28k forks source link

Caveat: $_ does not work well with a trap on DEBUG #60

Closed pawamoy closed 4 years ago

pawamoy commented 5 years ago

When a trap on DEBUG is set, $_ will always be the last argument of the last trap command, making it unusable. This could be noted somewhere in the README :slightly_smiling_face:

pawamoy commented 5 years ago

There's a workaround though (https://stackoverflow.com/questions/40944532/bash-preserve-in-a-debug-trap):

trapfunc() { local old_=$1; date; : "$old_"; }
trap 'trapfunc "$_"' DEBUG
georgalis commented 5 years ago

Timothée, perhaps communicate the desired and expected/actual behavior of your example? -George

On Sun, Apr 14, 2019 at 7:14 AM Timothée Mazzucotelli < notifications@github.com> wrote:

There's a workaroung though ( https://stackoverflow.com/questions/40944532/bash-preserve-in-a-debug-trap ):

trapfunc() { local old=$1; date; : "$old"; } trap 'trapfunc "$_"' DEBUG

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dylanaraps/pure-bash-bible/issues/60#issuecomment-482985060, or mute the thread https://github.com/notifications/unsubscribe-auth/AAm4Cz8SlkpxXZaMq8uX9wSqocJ0n3Taks5vgzfJgaJpZM4cdx5D .

-- George Georgalis, (415) 894-2710, http://www.galis.org/

pawamoy commented 5 years ago

Behavior without trap:

$ echo hello
hello
$ echo $_
hello

Behavior with a trap:

$ trap date debug
$ echo hello
Sun Apr 14 22:03:57 CEST 2019
hello
$ echo $_
Sun Apr 14 22:04:01 CEST 2019
date

In the last line, the user expected to get "hello", not "date".

Behavior with a trap, using the workaround:

$ trapfunc() { local old_=$1; date; : "$old_"; }
$ trap 'trapfunc "$_"' DEBUG
$ echo hello
Sun Apr 14 22:06:00 CEST 2019
hello
$ echo $_
Sun Apr 14 22:06:02 CEST 2019
hello
georgalis commented 5 years ago

So, "Behavior with a trap:" is what you don't want or what you don't expect?

In any event you may have a smoother future with

trapfunc() { local old="$*"; date; : "$old"; }

On Sun, Apr 14, 2019 at 1:09 PM Timothée Mazzucotelli < notifications@github.com> wrote:

Behavior without trap:

$ echo hellohello $ echo $_hello

Behavior with a trap:

$ trap date debug $ echo helloSun Apr 14 22:03:57 CEST 2019hello $ echo $_Sun Apr 14 22:04:01 CEST 2019date

Behavior with a trap, using the workaround:

$ trapfunc() { local old=$1; date; : "$old"; } $ trap 'trapfunc "$_"' DEBUG $ echo helloSun Apr 14 22:06:00 CEST 2019hello $ echo $_Sun Apr 14 22:06:02 CEST 2019hello

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dylanaraps/pure-bash-bible/issues/60#issuecomment-483053362, or mute the thread https://github.com/notifications/unsubscribe-auth/AAm4Cz-ZuesMsDNs8L1WRg6XWhcu7Jbnks5vg4rdgaJpZM4cdx5D .

-- George Georgalis, (415) 894-2710, http://www.galis.org/

pawamoy commented 5 years ago

What I recommend is simply a note in the README that explain this limitation / behavior.

And I think using "$*" instead of $1 is wrong, because you now return all the previous arguments as one string, instead of only the last. "$@" would be better, but it's not really useful either. It also depends on how you set your trap.

pawamoy commented 4 years ago

Are you still interested in an additional note in the TRAPS section of the README? I can send a PR. Otherwise I'll close this issue :slightly_smiling_face:

georgalis commented 4 years ago

When writing doc, it's important to understand the reader may have a different objective and they definitely have a different perspective. So there is a good chance the perceived "solution" is probably different too. I'm interested in your function, but your comments assume my objective and perspective is the same as yours. The following goes a long way to transparency and facilitates collaboration. Communicate:

Application of $@ $* $1 and quoting nuance, first depends on what you want, if you don't characterize the desired utility and objective, how can anyone tell if your solution is better than the actual behavior.

If you assume the actual behavior is expected, and explain why your patch is an improvement, it helps the reader understand you; and, a great side effect is, meaningful feedback such as "why the actual behavior is desirable," becomes possible.

pawamoy commented 4 years ago

Well, the discussion label was removed to leave only the bug one, so I thought this was clear, but I'll try again to explain.

$ echo hello
hello
$ echo $_
hello
$ echo hello
hello
$ echo $_
hello  # OK
$ trap date debug
$ echo hello
Sun Apr 14 22:03:57 CEST 2019
hello
$ echo $_
Sun Apr 14 22:04:01 CEST 2019
date  # user was probably expecting "hello", not "date"

Now, if you don't think this behavior is usually surprising, and that most of the users would understand why $_ equals date instead of hello, then let's close this issue. But when I imagine myself working on a big script, and everything works fine, but suddenly I get weird bugs because I added a trap on debug somewhere in the code, I know I would have wanted to know about this behavior with $_/traps, because debugging could be hard.

I hope it's a bit more clear.

Please note that I don't mind closing this. I just wanted to update the status on this so I could either send a PR or close the issue :slightly_smiling_face:

Now for the workaround, that could be added as well if this behavior is described in a note or caveat:

trapfunc() {
  local old_=$1  # remember the passed arg
  date  # trap contents, can be whatever
  : "$old_"  # store the remembered arg in $_ again
}

# each time the trap is executed,
# $_ will be passed to the function so it can restore it at the end
trap 'trapfunc "$_"' DEBUG

Note that using $* or $@ would not be correct here. $_ is one argument only. We pass it to the function, it receives it in $1, and stores it again in $_ at the end.

pawamoy commented 4 years ago

I don't think I can write a better explanation for this. I'm going to close, but feel free to re-open!