ohmybash / oh-my-bash

A delightful community-driven framework for managing your bash configuration, and an auto-update tool so that makes it easy to keep up with the latest updates from the community.
https://ohmybash.github.io
MIT License
5.54k stars 626 forks source link

themes/powerline: Added hex color capacity and user info text color #510

Closed GiulianoWF closed 6 months ago

GiulianoWF commented 6 months ago
GiulianoWF commented 6 months ago

Thanks for the feedback! I've updated the PR with the suggested changes.

Regarding the powerline-naked theme, I've added a new variable as per the discussion. It's currently not active due to the way the background color parameter is utilized.

GiulianoWF commented 6 months ago

I wanted to let you know that I've amended my last commit because I forgot to update a variable name. I wasn't entirely sure about the process and ended up force-pushing the change. I now understand this might not be the best practice, and I apologize for any mix-up it has caused.

If this is an issue, I'm more than willing to close this PR and open a new one with a fresh branch to keep things clean. Just let me know what works best for the project.

Thank you for your understanding.

akinomyoga commented 6 months ago

Don't worry about it.

Actually, force-pushing of organized commits is more welcome than trivial commits going back and forth. PRs with good quality would be developed by the discussions, where additional changes are typical. Meanwhile, for later investigations into the background of implementation details, it's good to have an organized history with each commit containing a related set of changes. It might depend on the project, but I prefer to finally have reorganized commits with rebasing/force-pushing/etc in PRs.

Anyway, this time, I'll probably squash all of the commits finally since this introduces a new theme. It's different from a PR containing multiple independent fixes to an existing one.

akinomyoga commented 6 months ago

Anyway, this time, I'll probably squash all of the commits finally since this introduces a new theme. It's different from a PR containing multiple independent fixes to an existing one.

Ah, sorry, this is not a new theme. I've mistaken it with another PR, #509, which tries to introduce the powerline-icon theme. This PR actually contains two independent features, the RGB color support and the specification of the user-info text color. However, these two changes are already mixed in the initial commit, so I wouldn't try to separate it into two commits, i.e., I'll squash them.

(If you would like, you can squash the commits and again split it into two for the RGB color and the user-info text color. In such a case, please force-push them to this PR rather than creating another PR, for the continuation of the discussion.)

GiulianoWF commented 6 months ago

Ok, done the changes.

GiulianoWF commented 6 months ago

And done

akinomyoga commented 6 months ago

I've merged the PR. Thank you!