Re: [PATCH] Completion batch #2: Misc. trivial fixes

[dana <dana@xxxxxxx>]

On 3 Jan 2018, at 17:40, Oliver Kiddle <okiddle@xxxxxxxxxxx> wrote:
>Digging around in $_cmd_variant is essentially looking into the
>internals of _pick_variant. The documented interface is to use the -r
>option to _pick_variant. Also, it is not saving the full output of
>expand --version, it will either have the value "gnu" or "unix".

Well, maybe it's worth bringing this up now, then, before i submit any further
patches: The reason i'd added that check is that i wanted to be able to complete
commands like this:

  % busybox expand -<TAB>

This use case doesn't seem (necessarily) compatible with _pick_variant as-is,
because your unadorned `expand` may be a totally different variant from `busybox
expand`. The way i had handled this in the _busybox function is:

  _cmd_variant[${words[1]}]=busybox _normal

That way you can temporarily override what _pick_variant thinks the actual
variant is. This seems to work quite well, but i did feel some guilt about it,
since as you mention it's circumventing the interface.

Another issue i had with _pick_variant is dealing with risky commands. In most
cases i imagine it's probably fine to do something like `poweroff --help`... but
i certainly didn't want to take the chance, since a badly written poweroff might
just kill the machine if you're running as root. So i had again bypassed
_pick_variant and manipulated _cmd_variant myself.

Do you have a suggestion as to how i could accomplish things like this in a less
hacky way? Maybe _pick_variant could learn an option to set the variant
directly? Alternatively, maybe i am over-engineering everything again...?

On 3 Jan 2018, at 17:40, Oliver Kiddle <okiddle@xxxxxxxxxxx> wrote:
>Given that these are hidden options, excluding other numeric options is
>pointless. It is also arguably wrong because the tab width can be more
>than 9 characters wide: e.g. expand -20 is valid.

All i really wanted was to have it not offer -t if a numeric option's already
been given. The completion function doesn't have to know that -20 is different
from -2 -0 to do that, AFAIK. Didn't consider that excluding the other options
is pointless though, i suppose it is superfluous.

Thanks!

dana