fix: add fixes for localvar_inherit

[akinomyoga]

I think this is not the complete list of the places that need a fix for localvar_inherit, but this pending branch starts to conflict with others frequently. I'm not planning to soon scan through all the codebase, so I decided to submit it as is now.

[scop]

Looks good in a way to me, but to be honest, having to add one line per such case along with a comment why it's done feels worse than if we'd instead initialize to the empty string on define and test with [[ $foo ]].

What do you think?

[akinomyoga]

I'm just afraid that it might break functions that would like to distinguish an empty value ''and the unset state. I can carefully check one by one if each function wants to distinguish an empty value and the unset state or not. Let me see...

[akinomyoga]

I've checked each of the codes that use local var; ...; [[ -v var ]]. In checking them, I found threefour suspicious places so first fixed them (b1ffcfb, 91e29fb, a13d1f1, a03809b). Then, I rewrote the codes in 7a4dcfe and 29007b5.

[scop]

Nice, looks better to me now.

A couple of points about the $has_ variables (no need to block this one for that, we can look into addressing them in followup PR's if we like). Leaving up to you to decide if we postpone them or do some of it here.

1. I suppose many of the cases where a $has_* was introduced here are because of bugs in existing implementations that fail to consider the empty string appropriately. Would be good to clean those up.

2. We have some precedent in _split_longopt and $split (and some other cases I think) in making vars used for booleans to have true or false values which can be tested by executing instead of doing the variable emptiness check (i.e. testing e.g. if $foo; ... instead of if [[ $foo ]]; .... I think that reads cleaner, we might want to have a look at doing that for the cases remaining if/after we weed out the cases where the separate boolean is not required in the first place.

[akinomyoga]

1. I suppose many of the cases where a $has_* was introduced here are because of bugs in existing implementations that fail to consider the empty string appropriately. Would be good to clean those up.

Currently, I do not have a good idea of how to handle these cases, so I think I'd postpone the resolution of these cases.

The remaining $has_* are all related to empty arguments stored in words, which originally come from COMP_WORDS. We usually don't have an empty element in COMP_WORDS (e.g., the empty argument '' or "" is usually stored in COMP_WORDS literally as '' or ""), but COMP_WORDS can still contain empty elements in some cases (e.g., cmd arg [TAB]). Having an empty element in words itself is not a bug because the empty element is coming from the user content, and we cannot output error or warning messages in the middle of completions when we find the empty element.

---

2. We have some precedent in _split_longopt and $split (and some other cases I think)

These are the other cases (excluding $split):

./bash_completion:823:    if $deduplicate; then
./completions/export:55:            if ! $remove && [[ $action != function ]]; then
./completions/find:74:    if ! $exprfound && [[ $cur != [-\(\),\!]* ]]; then
./completions/fio:121:                        if $in_values; then
./completions/jarsigner:43:    if ! $jar; then
./completions/lzip:38:    if $decompress; then
./completions/pack200:56:    if ! $pack; then
./completions/pack200:68:    elif ! $jar; then
./completions/pytest:115:            if $in_class && [[ $line =~ ^[[:space:]]+(async[[:space:]]+)?def[[:space:]]+(test_[A-Za-z0-9_]+) ]]; then
./completions/route:23:        $found || COMPREPLY+=("$opt")
./completions/ssh:490:    if $dirsonly; then
./completions/timeout:11:            if $found; then
./completions/unpack200:36:    if ! $pack; then
./completions/unpack200:44:    elif ! $jar; then

Another approach seems to prepare a variable that has a value 0 or 1, and test it with ((var == 0)), etc.

./completions/mcrypt:56:                decrypt=1
./completions/postcat:22:        [[ $idx == -q ]] && qfile=1 && break
./completions/svcadm:32:        exact_mode=1
./completions/tar:174:        old_opt_progress=1
./completions/tar:179:        old_opt_used=1
./completions/tar:204:    first_arg=1
./completions/wodim:72:            track_mode=1

The other boolean flags have been assigning a non-empty value and testing it with [[ -v $var ]], which were now converted to [[ $var ]]. This is the approach that I used for these new has_* variables.

./bash_completion:1909:            4) ipv4=1 ;;
./bash_completion:1910:            6) ipv6=1 ;;
./completions/tar:694:    basic_tar=yes
./completions/upgradepkg:20:        [[ ${COMPREPLY-} ]] || nofiles=1

[akinomyoga]

I have switched has_xxx to use true / false and "$has_xxx" / ! "$has_xxx" (cfe3487b..ace9cd66). I kept other existing cases.