search

caiogondim / bullet-train.zsh

:bullettrain_side: An oh-my-zsh shell theme based on the Powerline Vim plugin
MIT License
2.82k stars 383 forks source link

change color combination to fix bad contrast #107

Closed henry40408 closed 8 years ago

henry40408 commented 8 years ago

quick fix of #106

The combination I used comes from prompt_virtualenv, so I change it as well.

The color schema I currently use: Tomorrow Eighties

caiogondim commented 8 years ago

Can you rebase your branch so i can merge it?

henry40408 commented 8 years ago

@caiogondim sorry for messing up, now the branch is able to be merged.

henry40408 commented 8 years ago

108 has been fixed.

caiogondim commented 8 years ago

I cannot test anymore. sleep 6 is not working to show the segment. And cmd_elapsed is empty for me.

Can you make sure it is working?

caiogondim commented 8 years ago

And there is a regression as well, as @dbkaplun commented on #108

henry40408 commented 8 years ago

this is the commit which the bug has been fixed: https://github.com/henry40408/bullet-train-oh-my-zsh-theme/commit/64a09b7049ec6b9510c26e1ce088cd64e0ba8c04

the following is a screenshot of bug-fixed version

001

does "regression" mean that I should not change the color combination? sorry for my bad English

caiogondim commented 8 years ago

@henry40408 You never have to ask sorry for mistakes while speaking your non-native language =) By regression i meant a bug; a commit that made a feature stop working (https://en.wikipedia.org/wiki/Software_regression).

I will test your branch again and will use for the coming days to check if everything is ok.

Will let you know.

caiogondim commented 8 years ago

Getting this error on my other laptop screen shot 2016-01-08 at 14 29 57 Using zsh 5.2 (x86_64-apple-darwin15.0.0)

henry40408 commented 8 years ago

@caiogondim it seems that add-zsh-hook is provided by a user-contributed plugin.

I upgrade my zsh to 5.2, remove all zsh plugins but the theme to reproduce the bug.

2016-01-09 12 28 56

To fix the bug, I replace them with built-in hook functions.

@dbkaplun is the same bug you encountered?

dbkaplun commented 8 years ago

@henry40408 I didn't encounter any bug, I just saw that your code replaces the color-configurable logic with older logic. Looks like a merge conflict was incorrectly resolved.

henry40408 commented 8 years ago

@dbkaplun you mean this commit https://github.com/henry40408/bullet-train-oh-my-zsh-theme/commit/3687f260c847deef98ba8892681dda68c2b75100?

dbkaplun commented 8 years ago

I don't know which commit it was but if you look at this PR's diff you'll see that $BULLETTRAIN_EXEC_TIME_BG and $BULLETTRAIN_EXEC_TIME_FG were removed (probably unintentionally).

henry40408 commented 8 years ago

@dbkaplun I remove them accidentally when refactoring, I added them back.

dbkaplun commented 8 years ago

OK. I don't have enough context on your changes so I can't review it in depth. Sorry bro.

caiogondim commented 8 years ago

Closing this PR since in the current code it's already fixed and the colours are now parameterised.