Local variables should not be identified in all-caps

[rpdelaney]

Per #bash on freenode and Greg's wiki, "By convention, environment variables (PATH, EDITOR, SHELL, ...) and internal shell variables (BASH_VERSION, RANDOM, ...) are fully capitalized. All other variable names should contain at least one lowercase letter. Since variable names are case-sensitive, this convention avoids accidentally overriding environmental and internal variables."

As a prelude to a patch addressing Issue #19, would you accept a pull request setting the local variables to lower case? In addition to eliminating the (admittedly low) risk of sexy-bash-prompt over-writing environment variables, it would facilitate support of color overrides through the environment (making clear which variables are dimensioned locally and which variables are imported).

[twolfson]

This does make sense but I am not sure it is worth the trouble. All variables inside of the script will be limited to the scope of the script; the only way they leave is if they are explicitly exported.

By the same coin, we should be worried about overriding any programs by occupying the lower case namespace.

# In an extreme case
git="\$"
git status

I think that as long as we namespace any external overrides for the script, then we should be fine (e.g. PROMPT_ in PROMPT_USER_COLOR).

[rpdelaney]

Well, the purpose of the convention is that since shell scripts should be using lower-case names for their own variables, you can't have a collision (without running some kind of weird multi-threading in the same shell I guess?). It's when you're not following the convention that collisions become bugs in your house, rather than the other guy :)

It's helpful that the variable would have to be exported before it affects other shells, but that's a small comfort since this script will theoretically be executed at the start of every shell. Like TERM, that makes it important to get this right, I think.

FWIW, it's my trouble to make the change -- and it's a pretty easy string substitution by my reckoning. It might be more trouble for me to keep track with everything in all-caps regardless of scope, than to perform the substitutions. If you really insist, I can use a namespace and cross fingers. It just seems like exposure to unnecessary risk without any payoff.

[twolfson]

In any case, we should be using a namespace when allowing script specific global variables.

I am more than confident in our ability to switch UPPER_CASE variables to lower_case. My concern is any unwanted side effects of this change. We might not spot any errors but that doesn't mean new issues won't be reported regarding the change.

I will re-open the issue for now but no style guide should be taken as gospel. Always apply a grain of salt and keep an open mind. There are many opinions and all of them are subjective.

https://www.google.com/search?q=shell+style+guide

[rpdelaney]

I agree completely with the broad philosophical points you're raising, which I take to be: (a) use a dedicated namespace for globals and (b) every change, however innocent looking, potentially introduces new bugs and other unexpected behavior.

Fun fact: the first hit on that search, Google's shell style guide, prescribes lower case for function and variable names, and uppercase delimited by underscores for constants and environment variable names. :cake:

My main drive in raising this issue is to check if I'd be wasting time building a branch for a conversion. :o) Would you prefer to have lower-case conversion in a separate PR, or conversion with global overrides in the same branch?

[twolfson]

The overrides and casing conversion should be treated as separate issues/opened in separate PRs.

[rpdelaney]

Aye sir!

[rpdelaney]

Done here: https://github.com/twolfson/sexy-bash-prompt/pull/21