Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Preserve existing DEBUG traps #39

Closed
brandonweeks opened this issue May 9, 2017 · 10 comments
Closed

Preserve existing DEBUG traps #39

brandonweeks opened this issue May 9, 2017 · 10 comments

Comments

@brandonweeks
Copy link
Contributor

Currently bash-preexec clobbers existing DEBUG traps when it is sourced. Would you be interested in a PR that moves the contents of trap -p DEBUG into a function and appends it to preexec_functions?

@dimo414
Copy link
Collaborator

dimo414 commented May 9, 2017

Similar support for PROMPT_COMMAND (appended to precmd_functions) would also be nice.

@rcaloras
Copy link
Owner

rcaloras commented May 9, 2017

the original PROMPT_COMMAND should be preserved as part of PROMPT_COMMANDstill.
https://github.com/rcaloras/bash-preexec/blob/master/bash-preexec.sh#L270

We could also move it to the precmd_functions array as well.

@dimo414
Copy link
Collaborator

dimo414 commented May 9, 2017

Ah, nice. Moving it to precmd_functions (if that's what's done for the DEBUG trap) would make sense assuming it wouldn't break anyone. Otherwise leaving it as-is is probably fine.

@dimo414
Copy link
Collaborator

dimo414 commented Jul 15, 2017

Currently bash-preexec clobbers existing DEBUG traps when it is sourced.

I'm actually not sure this is true; I found that sourced scripts can't override an existing DEBUG trap. Worse, it seems from within the sourced script there's no way to tell whether a DEBUG trap was already set or not. I'm not sure if this FR is possible.

@rcaloras
Copy link
Owner

@dimo414 I noticed this a little while back. The debug trap isn't actually set when sourcing the script. bash-preexec actually does a two part install where it hooks its install function first into PROMPT_COMMAND and then runs setup through the invocation of that function. e.g. https://github.com/rcaloras/bash-preexec/blob/master/bash-preexec.sh#L306. and https://github.com/rcaloras/bash-preexec/blob/master/bash-preexec.sh#L241

Original commit for it: 4744d7e

@dimo414
Copy link
Collaborator

dimo414 commented Jul 18, 2017

Well that's clever :) It would not have crossed my mind to try that. In that case it seems like it should be possible to add the trap to preexec_functions with something like:

preexec_functions+=("$(trap -p DEBUG | sed "s/[^']*'\(.*\)'[^']*/\1/")")

There's probably a better way to extract the DEBUG command, but that worked in a pinch. I take it you're open to a pull request adding something along these lines?

@rcaloras
Copy link
Owner

Of course! Please feel free to submit a PR 👍

@rcaloras
Copy link
Owner

Closing addressed by @dimo414

@rcaloras
Copy link
Owner

@dimo414 Realized this PR no longer preserves the second half of a user's PROMPT_COMMAND. i.e. anything after the bash-preexec install commands. This is due to preserving PROMPT_COMMAND when it's first sourced and not as bash-preexec is being installed during the session init.

# If there's an existing PROMPT_COMMAND capture it and convert it into a function
# So it is preserved and invoked during precmd.
if [[ -n "$PROMPT_COMMAND" ]]; then
eval '__bp_original_prompt_command() {
'"$PROMPT_COMMAND"'
}'

Any way that we can preserve any commands afterwards into precmd? e.g. a .bash_profile like

PROMPT_COMMAND="foo"
source bash-preexec.sh
PROMPT_COMMAND="$PROMPT_COMMAND; bar"

would preserve both foo and bar.

I was originally doing this by parsing out the entire install string and then putting the original prompt command back into PROMPT_COMMAND:
https://github.com/rcaloras/bash-preexec/pull/50/files#diff-9f42bfd9f684d2763a4d12cc481506a1L244

rcaloras added a commit that referenced this issue Aug 20, 2020
- Relaxes requirement that bash-prexec.sh needs to be included as last
thing in bash_profile.sh
- Preserves existing prompt command in PROMPT_COMMAND variable. May make
sense to still move this to a function in precmd.
- Should address #97 and #39
- Keeps existing traps preserved as preexec functions
- Removes trailing \n on PROMPT_COMMAND added for #58.
rcaloras added a commit that referenced this issue Aug 22, 2020
- Relaxes requirement that bash-prexec.sh needs to be included as last
thing in bash_profile.sh
- Preserves existing prompt command in PROMPT_COMMAND variable. May make
sense to still move this to a function in precmd.
- Should address #97 and #39
- Keeps existing traps preserved as preexec functions
- Removes trailing \n on PROMPT_COMMAND added for #58.
@rcaloras
Copy link
Owner

Trap is preserved and prompt command is fixed in 0.4.0.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

3 participants