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

Update readme status explanation with settings properties #281

Merged
merged 1 commit into from
May 12, 2016

Conversation

dahlbyk
Copy link
Owner

@dahlbyk dahlbyk commented May 4, 2016

Closes #131, following on from #202 and #221.

@paulmarsy @abbiekressner thoughts?

@abbiekressner
Copy link

Looks great, @dahlbyk! Thanks!

@dahlbyk dahlbyk merged commit ed4d292 into master May 12, 2016
@dahlbyk dahlbyk deleted the readme-settings branch May 12, 2016 20:18
@paulmarsy
Copy link
Contributor

Sorry for the slow reply... documentation update looks good, which made me look at the history of the GitPrompt and I like the additions and changes 😄

My only comment is the naming of the settings.. I originally went with 'symbol' in the name to help guide future development towards a single character and so smaller footprint for the prompt rather than text which while accurate about what it can contain doesn't convey the notion of keeping the prompt unobtrusive.

I don't mind either as it's common sense to avoid bloating the prompt, but the settings do have a mixture of both 'text' and 'symbol' terms across all the settings containing 1-2 characters of punctuation.

At this point in time it's likely harmful to align them to one or the other as people will have written code to look for those setting names, but thought I would give the reasoning why I used 'symbol' so the naming would be considered for any new settings.

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.

3 participants