Style overrides - Githubissues

twolfson / sexy-bash-prompt

Bash prompt with colors, git statuses, and git branches.

MIT License

1.14k stars 155 forks source link

Style overrides #22

Closed rpdelaney closed 10 years ago

rpdelaney commented 10 years ago

Application of optional color over-rides are added on line 55. Any over-ride that is unset will be ignored. The naming scheme is PROMPT_*

Sorry if the multiple diffs on .bash_prompt are hard to follow in the aggregate. Some notes that might help:

Because the default colors don't set the background color, I added several extra $resets to the PS1. After some testing I decided it was cleaner than setting the background color in the default palette.
But since tput sr0 un-sets boldface, I added $bold to the definitions of each item of the default colorscheme, so that the default behavior will stay consistent with the master branch.
Now, because I invoke $bold inside the default color scheme, I had to move the dimensioning of the "common color actions" up to line 17, so that they were already defined before we do anything else.
It seemed most appropriate to have the last instance of $preposition_color moved into the git status sub-clause. This is arguably needed in the master branch (with or without style overrides), but I included it in this pull request because it caused a specific problem when using a tput setbg on PROMPT_PREPOSTION_COLOR.
Note that this might mean that get_prompt_symbol no longer has preposition_color applied to it. I liked it fine this way, but if you want, I can add another invocation of preposition_color to correct that. Another option worth considering would be a separate variable for prompt_symbol_color or somesuch, so that it could be configured (and overridden) separately.

twolfson commented 10 years ago

Looks pretty good, left some notes.

rpdelaney commented 10 years ago

Ship it! :shipit:

rpdelaney commented 10 years ago

Did a rebase to clean up the development history. How does that look?

twolfson commented 10 years ago

I am not too worried about history. I rely on tags + changelog to analyze/navigate history.

The code looks fine. I will do a final run through by the end of the weekend (this week is a bit hectic for me).

twolfson commented 10 years ago

Taking a look at this now.

twolfson commented 10 years ago

This has been merged/released in 0.16.0.

rpdelaney commented 10 years ago

PROMPT_BOLD is necessary somewhere so that the user can decide not to bold stuff.

I haven't thought of any use for PROMPT_RESET, but also I couldn't think of a reason not to provide it either, in case someone did.

twolfson commented 10 years ago

If someone wants something to not be bold, then they can set each of overrides to their unboldened variant.

Additionally, the code in the PR meant that the prompt_bold would be set after the colors were already set. In the actual PS1, there was no mention of prompt_bold.

I have found that it is better to stick with the minimal set of features. This way you get a few things:

A new issue/discussion around the intent of what they are looking to do which could expand into something more than their one-off case (e.g. you and your root coloring)
Prevents overextending the "one thing well" purpose of this repo
- If someone wants something more full-featured, then they should either compose on top of this script or build something designed for that purpose
It adds unnecessary maintenance weight. If we add a feature that nobody is using/needs, why should we maintain it?

twolfson commented 10 years ago

Thanks again for the PR =)

rpdelaney commented 10 years ago

Fair enough, thanks.