Provide trap function for wrapping DEBUG signal traps

[aprescott]

Occasionally .bashrc (or similar) has an existing trap 'foo' DEBUG which can conflict with auto.sh's. I recently hit this problem since I noticed .ruby-version was being ignored.

The way I fixed this was to patch auto.sh to have:

function chruby_trap() {
    [[ "$BASH_COMMAND" != "$PROMPT_COMMAND" ]] && chruby_auto
}

if [[ -n "$ZSH_VERSION" ]]; then
    if [[ ! "$preexec_functions" == *chruby_auto* ]]; then
        preexec_functions+=("chruby_auto")
    fi
elif [[ -n "$BASH_VERSION" ]]; then
    trap chruby_trap DEBUG
fi

Then in my .bashrc, after loading chruby.sh and auto.sh, after any other calls, I provide my own trap:

# Reset color for command output and call this new `chruby_trap` function
trap 'foo_bar; chruby_trap' DEBUG

Obviously chruby_auto would also have worked, if I'd provided the explicit $BASH_COMMAND != $PROMPT_COMMAND check, but I figured it was possible that "setting up the trap" wouldn't necessarily always be the same as "call chruby_auto", hence the extra function.

I can submit a PR if you're happy with the idea (probably with refinements). Failing that, I can also submit a README note to mention that auto.sh will set a DEBUG trap and explain how to wrap it with chruby_auto.

[aprescott]

(README update would also happen if chruby_trap is implemented of course! Or a wiki entry. Whichever.)

[ilikepi]

I happened upon this issue in the process of writing up a similar proposal. I agree it would be beneficial for the call to chruby_auto to be wrapped in another function that could more easily be overridden. I was going to propose a slightly different change however:

diff --git a/share/chruby/auto.sh b/share/chruby/auto.sh
index 88f3cb3..e3ad1ee 100644
--- a/share/chruby/auto.sh
+++ b/share/chruby/auto.sh
@@ -1,5 +1,9 @@
 unset RUBY_AUTO_VERSION

+function ruby_version_autoexec() {
+   chruby_auto
+}
+
 function chruby_auto() {
    local dir="$PWD/" version

@@ -25,9 +29,9 @@ function chruby_auto() {
 }

 if [[ -n "$ZSH_VERSION" ]]; then
-   if [[ ! "$preexec_functions" == *chruby_auto* ]]; then
-       preexec_functions+=("chruby_auto")
+   if [[ ! "$preexec_functions" == *ruby_version_autoexec* ]]; then
+       preexec_functions+=("ruby_version_autoexec")
    fi
 elif [[ -n "$BASH_VERSION" ]]; then
-   trap '[[ "$BASH_COMMAND" != "$PROMPT_COMMAND" ]] && chruby_auto' DEBUG
+   trap '[[ "$BASH_COMMAND" != "$PROMPT_COMMAND" ]] && ruby_version_autoexec' DEBUG
 fi

My own motivation is that I have another function I want to call after chruby_auto. Initially I was sourcing a second file with a modified trap command, but the second call to trap was ignored unless I first removed the existing trap command. This is rather awkward and confusing; having a wrapper function I can override seems more straightforward to me.

I had some difficulty settling on a name for the wrapper function. I got it in my head that it might make sense to give it a generic-sounding name as opposed to using the chruby_* prefix. I don't have a strong argument to support this, however.

Naming aside, I think this makes it easier to augment chruby_auto's behavior without having to patch auto.sh. I'm hoping to find some support for this idea...

[HaleTom]

I also believe that chruby should not clobber an existing DEBUG trap, following the principle of least surprise.

This code allows extending an existing trap.

[FranklinYu]

@HaleTom Does that work with https://github.com/rcaloras/bash-preexec? Because I also wish chruby to work with Bash-Preexec (#399).

[postmodern]

Investigating adding additional hooks. Should we provide a way to load chruby_auto (or similarly named function) without setting the trap DEBUG hook so that one can set their own trap DEBUG? Or should we extract the logic in chruby_auto for explicitly loading the contents of a .ruby-version file?

[ilikepi]

It seems like the root issue here is the fact that auto.sh both defines the chruby_auto function and attempts to configure the shell to call it. I think the simplest way to address this would be to separate those two pieces of functionality. For example, definition of chruby_auto could be extracted to a different script file, and auto.sh could call that new script file and then move on to configuring the shell as it does now. (Maybe this is what you meant by "...extract the logic in chruby_auto for explicitly loading the contents of a .ruby-version file"?) Although this would add one more script file to the repo, it should address the issue without breaking any existing installation. Users who want to modify how chruby_auto is called could implement whatever solution works best after calling the new script file instead of auto.sh.

I think there's definitely potential value in providing pre- and post-execution hooks to chruby_auto and/or chruby. Maybe for the sake of reaching the 1.0 milestone, the best course of action would be to defer that new functionality for future releases?

[postmodern]

I'm messing around in the 1.0.0 branch with extracting the trap DEBUG/preexec_functions hook as a generic chruby_hook function that calls other functions added to chruby_hooks. This would allow adding other per-command hooks to auto-detect bundler. Is chruby_hook/chruby_hooks an appropriate name?