Bash completion nounset mode fixes

[scop]

I'm not sure if this is complete, but it addresses some low hanging fruits related to nounset mode bash.

[jpmcb]

Hi @scop - can you provide some further detail on the problem and what this fix accomplishes? I'm not sure there is a problem (or I just don't understand it yet!).

My understanding of the conditional -n is:

-n string
       True if the length of string is non-zero.

Regarding nounset, I can see this possibly being a problem if those variables are unset, are people running their shells in this fashion? Again, any further context on this would be really helpful!

If we do this, I think I would prefer to see this format:

if [ -n "${ZSH_VERSION:-}" ]

But let me know if that won't work!

[marckhouzam]

@scop Thanks for the contribution. Be aware however that most of these changes are already being addressed in #1321, maybe you can comment on that PR to add the missing parts?

[scop]

Regarding nounset, I can see this possibly being a problem if those variables are unset, are people running their shells in this fashion?

Yes. Mileages vary, but it's one good way of avoiding havoc caused by mistyping variable names.

If we do this, I think I would prefer to see this format:

if [ -n "${ZSH_VERSION:-}" ]

But let me know if that won't work!

It does work, but strictly speaking it's unnecessary/redundant: it substitutes not only unset values but also empty values with an empty value. The variant without the colon only substitutes unset values with an empty string, which captures the intent exactly without redundancy. Therefore I think that form is better, and also easier on the eye if you ask me ;)

[scop]

@scop Thanks for the contribution. Be aware however that most of these changes are already being addressed in #1321, maybe you can comment on that PR to add the missing parts?

Ah, I was unaware of it, should have done a search, sorry about that. However what I'd do with that PR is just replace it in entirety with the contents of this one; this one is more complete and in my opinion more "correct" stylewise as discussed above. But I'll add a note there.

[marckhouzam]

@scop After discussion in the other PR we determined that has_completion_function and last_command do not need this change but instead need to be initialized with an = sign.

[scop]

That is another way to fix that part of the problem. Again, stylewise kind of worse way if you ask me, but also again, I don't care too deeply about the "how" as long as the fixes end up in the tree in one form or another.

[scop]

That is another way to fix that part of the problem. Again, stylewise kind of worse way if you ask me [...]

On a 2nd thought, I'll call it a tie, because all existing variable "declarations" in the generated code are already initiliazing the variable to something, so for consistency it's ok to initialize these to the empty string.

[jpmcb]

Great discussion! I'll go ahead and close this and address the existing PR - thanks again for the contribution!!