fix(bash): last command status and command duration

[MunifTanjim]

Description

For bash,

Appending starship_precmd to PROMPT_COMMAND breaks previous command's status ($?) checking when other commands already exist in PROMPT_COMMAND. Because those commands can modify the actual $? variable.

Prepending starship_precmd to PROMPT_COMMAND breaks command duration module when other commans already exist in PROMPT_COMMAND. Because those commands can trigger setting the STARSHIP_START_TIME variable.

So, to fix those cases, existing PROMPT_COMMAND is preserved in _PRESERVED_PROMPT_COMMAND variable which is later executed inside the starship_precmd function.

Note: nojhan/liquidprompt also does the same thing.

• https://github.com/nojhan/liquidprompt/blob/5f4aeece8d6cf98138e729f7833e11e985ca44d3/liquidprompt#L1718
• https://github.com/nojhan/liquidprompt/blob/5f4aeece8d6cf98138e729f7833e11e985ca44d3/liquidprompt#L1891

Motivation and Context

Related to #954, which was closed without solving the underlying issue that caused it.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Screenshots (if appropriate):

How Has This Been Tested?

• I have tested using MacOS
• I have tested using Linux
• I have tested using Windows

Checklist:

• I have updated the documentation accordingly.
• I have updated the tests accordingly.

[MunifTanjim]

Currently, I'm doing this as a workaround:

eval "$(starship init bash)"
export PROMPT_COMMAND="starship_precmd;${PROMPT_COMMAND/starship_precmd;/}"

It adds the starship_precmd; command to the start of PROMPT_COMMAND and removes it from the end.

This workaround broke the "command duration" module.

[X-dark]

This breaks command duration for me as idle time become counted as well.

[MunifTanjim]

This breaks command duration for me as idle time become counted as well.

Strange. The workaround works fine. But the change in this PR breaks command duration. 🤔

@X-dark can you confirm if the workaround (without the change in this PR) works for you?

[X-dark]

@X-dark can you confirm if the workaround (without the change in this PR) works for you?

Sorry I have tested the workaround, not the PR.

Without any fix or workaround, my PROMPT_COMMAND looks like this:

__vte_prompt_command; printf "\033]0;%s@%s:%s\007" "${USER}" "${HOSTNAME%%.*}" "${PWD/#$HOME/\~}";starship_precmd;

[MunifTanjim]

Thanks @X-dark . I've updated the PR to handle those cases.

[chipbuster]

LGTM. This is really quite a clever solution, and simple to implement. Well done!

[chipbuster]

Thanks for the comment here--will really help in the future.

[chipbuster]

I have to speak frankly here for a second: the number of hoops we've jumped through on this project to try to get things working perfectly in Bash is starting to get ludicrous. First we set, then we appended, then we prepended. We've changed separators between semicolons and whitespace, we've had to deal with other shell helpers breaking starship with their own initscripts. Every single time, I've been optimistic that this change will finally fix everything, and every time, I've been proven wrong. And judging by the number of conflicts we've gotten with other shell frameworks, I'm not sure they're doing much better than us.

I certainly hope that this will fix our problems once and for all (and looking at it, I do think it will!), but in the event that I'm proven wrong again, I'm going to have to suggest that we push that responsibility on the user in one of two ways:

• Request that they install bash-preexec, which abstracts away this problem and we already have support for.
• Force the user to decide their own ordering: make an init flag for bash only (e.g. starship init bash --funcs-only) which will only define starship_precmd and starship_preexec, and decide that it's the user's responsibility to figure out how they want their hooks ordered.