Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Incompatible prompt_command settings #58

Closed
jombooth opened this issue Oct 5, 2017 · 2 comments
Closed

Incompatible prompt_command settings #58

jombooth opened this issue Oct 5, 2017 · 2 comments

Comments

@jombooth
Copy link
Contributor

jombooth commented Oct 5, 2017

Some users of a project I work on that's based on Bash-Preexec modify their prompt command in ~/.bashrc after sourcing the script. In some cases, we've found they'll do this:

PROMPT_COMMAND+="history -a"

This breaks BP, because prompt_command ends in "__bp_install", so on first execution, the user gets a
"no such command: __bp_installhistory" message.

To fix this particular issue, I suggest wrapping the prompt_command installed by bash_preexec in newlines, ie
__bp_preexec_foo_bar; __bp_install \n

so that appending to PROMPT_COMMAND won't cause problems. Newlines are in general the most universally compatible way of splitting commands in Bash, so we could consider using them wherever a problem might arise, ie,

\n __bp_preexec_foo_bar \n __bp_install \n

This way, even if someone does a PROMPT_COMMAND="echo good morning! $PROMPT_COMMAND", after sourcing bash-preexec, the result will still be correct.

w.r.t. testing this - it would be ideal to test various possible orderings of setting up custom prompt commands and sourcing Bash-Preexec, for example,

adjust PROMPT_COMMAND
source bash_preexec
adjust PROMPT_COMMAND again

source bash_preexec
adjust PROMPT_COMMAND

@rcaloras
Copy link
Owner

rcaloras commented Oct 5, 2017

Yeah, there used to be semicolons, but we decided to remove them. #41

Newlines sound more forgiving :) want to submit the PR?

rcaloras added a commit that referenced this issue Oct 18, 2017
* Add newlines to PROMPT_COMMAND install string for #58

* Updating to use newlines
@rcaloras
Copy link
Owner

@jombooth closing this guy since I merged the PR. Hoping this is fixed now on master.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants