canonical / multipass

Multipass orchestrates virtual Ubuntu instances
https://multipass.run
GNU General Public License v3.0
7.88k stars 651 forks source link

[macos] bash completion seeing "-bash: COMP_WORDS: bad array subscript" #2237

Closed townsend2010 closed 3 years ago

townsend2010 commented 3 years ago

For bash completions on macOS, I'm seeing the following when trying to tab complete everything: -bash: COMP_WORDS: bad array subscript

It seems the completions still work, but this is thrown after tabbing.

townsend2010 commented 3 years ago

Ok, I found that https://github.com/canonical/multipass/blob/main/completions/bash/multipass#L133 is the offending line and was introduced in commit https://github.com/canonical/multipass/commit/fa575b934.

luis4a0 commented 3 years ago

Thanks @townsend2010! It happens I didn't check that an array index was not negative. I am using bash 5 and it silently skips the error. But bash 3 (default in macOS) complains. Adding a simple check does not hurt.

luis4a0 commented 3 years ago

Something more: bash 3 does not have compopt, which is used later in the completion script. It was introduced in bash 4. We need to require bash 4 to run the script. How can we enforce having bash>4?

Note: bash 4 is from 2009, any modern system must have it already installed.

townsend2010 commented 3 years ago

We need to require bash 4 to run the script. How can we enforce having bash>4?

I don't think we can enforce this since this is what is in macOS 10.x. Since this is only applicable to the --network option in launch, I wonder if it would be better just not to support --network on macOS until we can find an alternative way of supporting that completion.

The other thing would be to make the completions work only on zsh on macOS and drop bash support itself.

townsend2010 commented 3 years ago

Thinking about it a bit more, there is also the alternative of newer bash in brew, but again, that would require detecting the bash version and either not installing the script on <4 or somehow have the completions show some message stating as much.

luis4a0 commented 3 years ago

I don't see as a good choice to not install the script. If the user updates bash later, then he/she would profit from the script. Maybe detecting the bash version at script startup and simply act as no-op in bash < 4 seems to be reasonable to me.

luis4a0 commented 3 years ago

@townsend2010 Done, you can take a look in #2238.

luis4a0 commented 3 years ago

@townsend2010 Thinking and rethinking. We do a shell/version check (detect zsh/bash and its versions) and, based on that check, decide whether to complete --network. It's a simple runtime check, which sets a variable.WDYT?

townsend2010 commented 3 years ago

Hey @luis4a0,

I think it would be better to detect shells and versions and selectively disable options that require newer versions or are shell specific. I think it's kind of heavy handed to not use any bash completions for users using the default version of bash on macOS.

luis4a0 commented 3 years ago

Oh, yes, it seems we typed toghether :)

Let's do that!