More configurable colors that follows the original pure colors

[boyeln]

related: #69

---

I changed the prompt symbol color to the one used in the original pure theme (ref: #69). I kept the gray color used, as the one used in the original (242) was quite unreadable in my opinion. I also created color variables for every event/attribute/symbol used, to make the theme more configurable.

I can split this pull request in two if desired; one for the color change of the prompt symbol (057e8fc and 7a19f21), and one for the new color variables (e211aa6 and cebf84e).

[rafaelrinaldi]

Thank you for this! No need to create two separate PRs but can you please take a look at why the tests are breaking? Feel free to edit/add to them if necessary.

[boyeln]

@rafaelrinaldi Yes, I have no idea why they did not work, seems like Travis timed out. Created a new branch(link) that's an exact copy of this branch, and the tests passed in that branch (link). Seems like something similar happend to #68 (link). Perhaps there is something funky with the travis configuration? I cannot see anything, but maybe it will run more smoothly if the dist option is removed, as it seems like this is where it halts? Tested that here, and it worked fine. I will try to force travis to run again by re-commiting my last commit.

[boyeln]

Ok, so the test passed after re-committing the last commit (creating a new sha by running git commit --amend --no-edit when even with origin/master, followed by git push --force origin master). So there seems to be some instability with the current travis configuration. I have never experienced this before.

[boyeln]

The issue with the first travis build seems to be the dist: xenial option within the travis configuration. See #71

[rafaelrinaldi]

Thanks a lot for your effort here. I will take a better look at everything this weekend once I have time and will get back to you.

[ntwb]

This is missing from the changes: set pure_color_green (set_color "66ff66")

Keeping them in alphabetical order rather than the random order chosen above would also be 👌

[boyeln]

Thanks for the feedback 🎉 Changing the order of the colors to be alphabetical is a great idea! The green color isn't included because it does not exist any more, as it's no longer used (replaced by the magenta color)

[ntwb]

I see plenty of green in the original pure theme screenshot:

More generally green is a pretty common color (e.g diff outputs) and should 100% remain as a customizable option, I I'm not the only user who uses a custom value for pure_color_green

[boyeln]

The green in the screenshot is not from pure. Pure is only the prompt, i.e. the stuff before what you type in the terminal. What comes after is dependent on other plugins you use. The green seen in that screenshot is caused by the hyper-snazzy plugin.
The only place this version of pure uses green, in the current master branch, is for the prompt symbol ("❯"). This is magenta (as in the screenshot) in the original pure theme. If you would like to change that, you should change the pure_color_success I think.

[boyeln]

The name of the variable pure_color_success could perhaps be renamed to pure_color_prompt_symbol, and the pure_color_error to pure_color_prompt_symbol_error, if that makes it more readable/understandable?

[andreiborisov]

@ntwb To customise fish colors use fish_config colors or set special fish color variables https://fishshell.com/docs/current/index.html#variables in your config.fish

[andreiborisov]

@boyeborg I would also propose to change pure_color_gray to brblack terminal color. I mean we are using terminal colors for all elements except this one, seems inconsistent to me. Also we shouldn't make any assumptions of user color scheme anyway, or otherwise it won't work for some users, right now if you have dark grey background, this color can be indistinguishable from it as a result of hardcoding color value. We should delegate this issues to theme plugins and terminal color schemes. And if user wants to customise it we'll have plenty of options thanks to this PR.

What do you think @edouard-lopez?

[rafaelrinaldi]

@boyeborg It seems like everything should be good to go to merge this in as well?

[boyeln]

@rafaelrinaldi Yes, I rebased to match the latest changes in master, so should be good now

[adriantre]

Looking forward to this 💖

[rafaelrinaldi]

Will do my best to launch in the upcoming days as soon as I have some time left.

[edouard-lopez]

@boyeborg Would you be kind enough to extract the code related to git (L96→L142) in its own function in order to improve maintainability and help us test the project?

[boyeln]

@edouard-lopez Sorry for the delay, I'll try to fix it this weekend

[rafaelrinaldi]

@boyeborg Have you had the chance to look into this? Maybe we can jump into to help get this out.

[andreiborisov]

@boyeborg Have you had the chance to look into this? Maybe we can jump into to help get this out.

@rafaelrinaldi I’ve rebased it and made a couple of additional tweaks: #103

[edouard-lopez]

@boyeborg I just merge #96 to introduce a number of tests, could you rebase and update your code and tests?

Let me know if you need help with the tests

[andreiborisov]

@edouard-lopez done here

[boyeln]

Sorry for the hold ut, I recently partnered up with a kitten who needed some love. Thanks for continuing the work @schrodincat, I suggest closing this PR and merge #103.

[edouard-lopez]

Thanks for the update 👍