Ensure that the format is reset within the git subshell rather than after it.

[jservice-rvbd]

I would like to source your bash prompt and then adjust PS1 after the fact.

I tried to remove the newline and instead have brackets around everything before the prompt symbol by setting PS1="[${PS1/\\n/] }" but this resulted in the left bracket coming out in the default text colour and the right bracket coming out in sexy_bash_prompt_preposition_color.

I'm not sure what the point of the sexy_bash_prompt_preposition_color immediately following git progress is; it doesn't seem to be doing anything.

[twolfson]

Preposition color is typically used for works like at/in. We're using it in the last line to reset colors whereas reset is used to reset boldening (I know they aren't particularly intuitive)

Unfortunately, we won't be accepting this PR as it affects the existing setup. For customizing the shell in a non-extensible fashion, we recommend either:

• Redefining PS1 after sourcing .bash_prompt
• Forking our repo and using the fork

[jservice-rvbd]

Hi. I don't mean to argue, I just want to make sure my original point was understood. Please correct me if I'm wrong.

My original point regarding sexy_bash_prompt_preposition_color was that its use in line 270 specifically didn't seem to have a point. The format of PS1 just before the subshell is opened on line 266 is sexy_bash_prompt_reset, as it will be if sexy_bash_prompt_is_on_git returns false. If it returns true, however, the format will be set to sexy_bash_prompt_preposition_color before the subshell exits. The only thing between the closing of the subshell and the next reset is the newline, so it doesn't actually make a difference, in which case the setting of sexy_bash_prompt_preposition_color seems superfluous.

Again, apologies if I'm reading this wrong, or if you understood my point but have closed the PR for another reason.

[twolfson]

The subshell echoes exit codes which adjust PS1's color as it outputs. The terminal emulator doesn't know about subshells, it only looks at the string generated in PS1 and the exit codes in said string.

As a result, exiting the subshell without outputting a reset code for the color (i.e. preposition_color) would leave the remainder of the text the same color as git_progress_color

For example:

echo "$(echo -n "blue")"foo
# Outputs: bluefoo
# Terminal sees: blue, then foo so "foo" would be blue

vs

echo "$(echo -n "blue" && echo -n "white")"foo
# Outputs: bluewhitefoo
# Terminal sees: blue, then white, then foo so "foo" would be white

Also as mentioned before our reset variable is only for resetting bold, not colors =/

[jservice-rvbd]

Okay, I think I understand the problem we're having. It's my understanding that tput sgr0 and \033[m reset all attributes, colour included, so your reset variable does reset colours. That certainly seems to be the case on both my mac and linux terminals.

[twolfson]

Ah, interesting. I don't currently have the time to fully look into this again. Here's the original PR that introduced it:

https://github.com/twolfson/sexy-bash-prompt/pull/22

It's mentioning background colors as well. Maybe that's what the reset variable was handling (in case of overrides)?

[rpdelaney]

It's been awhile now but IIRC I wanted the reset variable to reset everything (including backgrounds) in case there were background overrides elsewhere in the user's configuration, yeah.

[jservice-rvbd]

It does seem that the reset variable does and was meant to reset and everything, in which case I think my change is correct. I believe I have also now fixed the tests in my branch, so it's there if you want it.

[twolfson]

I'll take a look at this again by the end of the weekend

[twolfson]

Alright, I was able to confirm that our reset resets everything one would expect (e.g. foreground, background, bold). I'm guessing we had human error in #22. Sorry for the trouble @jservice-rvbd =/

Ironically, I found a new bug while doing my exploration. We don't have reset at the head of the PS1 so background colors bleed over into the first item:

@jservice-rvbd Could you add in a fix to this PR for that as well? Also, I think you'll need to update the tests to get this PR to green

[rpdelaney]

Earth to @jservice-rvbd ! Are you still interested in this PR?

[jservice-rvbd]

@rpdelaney Whoops. Completely forgot about this, sorry.

[twolfson]

I've been on vacation for a week and am catching up on things. I'll review this by the end of the weekend

[twolfson]

Looks good to me. Thanks for the PR @jservice-rvbd ! Releasing this now