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

[Oliver Kiddle <okiddle@xxxxxxxxxxx>]

dana's message doesn't seem to have come through on the mailing list so
I'll quote generously.

dana 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.

Given that that works well, I'd go with that solution. It'd be possible
to add an option to _normal/_dispatch. Having them know about
_pick_variant internals is less ugly. It is, however, probably only worthwhile if
it is going to be used elsewhere. It might be wise to dig around for
other use cases. _builtin is the only thing that comes to mind.

> 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.

Yes, we don't want to ever call risky commands or commands that might
have side-effects. For poweroff, just don't use _pick_variant. Rely
on $OSTYPE and other tests like [[ -d /etc/systemd ]]. Caching the
result is unlikely to be necessary but if it is, don't use _cmd_variant.
poweroff likely doesn't get run many times in a single zsh session
anyway.

> 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.

Doesn't "superfluous" have much the same meaning as "pointless" in this
case. It seems to work as it is but I'd clean it up if making changes
for the busybox variant anyway. Doesn't busybox allow expand to be a
hard link to busybox to run expand?

I've applied the rename patch by the way. Bart makes a good point and I
also prefer _file_modes to _fmodes.

Oliver