Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable colours #39

Merged
merged 21 commits into from
Sep 4, 2015
Merged

Configurable colours #39

merged 21 commits into from
Sep 4, 2015

Conversation

michaeldfallen
Copy link
Owner

This pull request provides the ability to modify the colours used for the various parts of the prompt. It was requested in issue #12. It provides the option of using either ENV vars or an rc file at ~/.gitradarrc.

The only thing missing at the moment is documentation but it's 1am here so I'll do it in the morning.

@apas, @jackmaney, @jedsmith, @Droogans: code review?

@jackmaney
Copy link

I have over four hours of meetings today (send help!), so I don't think I'll get to a code review today, but I'll look it over when I can. Thank you for your work on this.

@Droogans
Copy link

Looks like a good starting point, but can you include the branch name as configurable too? All the other colors render fine on a default instance of "terminal" (Pro theme) for Yosemite, except that the branch name is as black as the background.

I'm not sure why the demo shows that the branch names are grey in the README. I'd really like to make them white.

@michaeldfallen
Copy link
Owner Author

@Droogans I did change them to the default color, which would be white, in 670471c. I just haven't got around to taking fresh screenshots (they take a while).

You're right, the branch name should also be configurable. I'll add that in.

@michaeldfallen
Copy link
Owner Author

Ok, that last commit adds the ability to configure the colour of the branch reference and the symbol used to represent master (which I set as default as an italic m, just cause I like it :trollface:). Time for documentation and then I'll merge.

@Droogans
Copy link

Droogans commented Sep 1, 2015

Excellent!

The only enhancement I can think of that works with the system you're building is to (later, of course) expose some generic themes. A light and a dark theme that work well for folks who are using a typical white on black/black on white theme. I imagine most are.

That way you can skip the configuration file entirely and swap it out with a command line arg.

export PS1="$PS1\$(git-radar --bash --theme=dark )"

@apas
Copy link

apas commented Sep 1, 2015

Hey -- didn't have time to do any code review but browsed around some of the updated files. I'll take @Droogans word it's ok! 😁 👍

@michaeldfallen
Copy link
Owner Author

Cool, I'll write some docs today and merge it.

michaeldfallen added a commit that referenced this pull request Sep 4, 2015
@michaeldfallen michaeldfallen merged commit 18bbc52 into master Sep 4, 2015
@michaeldfallen michaeldfallen deleted the configurable-parts branch September 4, 2015 11:31
@michaeldfallen michaeldfallen mentioned this pull request Sep 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants