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

fix(bash): Restore previous exit status in bash init #3521

Merged
merged 2 commits into from Jan 30, 2022

Conversation

chipbuster
Copy link
Contributor

Description

Restore the previous exit status so that any subsequent hooks in the PROMPT_COMMAND pipeline can run with the correct return code.

Motivation and Context

Closes #2918

Screenshots (if appropriate):

How Has This Been Tested?

I've confirmed that this doesn't negatively affect bash init, but I haven't confirmed that this actually causes the behavior intended. The fix is based on a comment by lilyball on the linked issue.

Checklist:

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

@chipbuster
Copy link
Contributor Author

I can't request changes on my on my own PR, but I can merge it without an approving review? What??

Anyways, some testing has revealed that this does not entirely solve the linked issue, so let's not merge this until I've had a chance to test it more thoroughly.

Copy link
Member

@andytom andytom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking for now

@andytom
Copy link
Member

andytom commented Jan 26, 2022

I can't request changes on my on my own PR, but I can merge it without an approving review? What??

Anyways, some testing has revealed that this does not entirely solve the linked issue, so let's not merge this until I've had a chance to test it more thoroughly.

Ooo interesting what are the problems it seemed to work fine for me locally, I only did a wee bit of testing so maybe I missed something.

@chipbuster
Copy link
Contributor Author

Here's what I used to test, starting up from a shell with (almost) no config:

derp(){ echo "Previous exit was $?"; }
PROMPT_COMMAND=derp

# Separate commands, should result in exit codes of 0, 1, and 127
true
false
doesnotexist

eval "$(starship init bash)"

true
false
doesnotexist

With the previous version, after starship init bash, on my system, I would always get "Previous exit was 0".

Shell version is GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu)

@chipbuster
Copy link
Contributor Author

Okay, I think this fixes the problem I saw.

@andytom andytom changed the title fix: Restore previous exit status in bash init fix(bash): Restore previous exit status in bash init Jan 29, 2022
@andytom andytom merged commit 6e24358 into starship:master Jan 30, 2022
Perniciosius pushed a commit to Perniciosius/starship that referenced this pull request Feb 21, 2022
* fix: Restore previous exit status in bash init

* Do it correctly this time
@chipbuster chipbuster deleted the prev_exit_status branch May 18, 2022 04:25
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

Successfully merging this pull request may close these issues.

starship_precmd should restore previous exit status
2 participants