yubikey-manager icon indicating copy to clipboard operation
yubikey-manager copied to clipboard

Add shell completion for --device and --reader

Open emlun opened this issue 4 years ago • 2 comments
trafficstars

(Experimental/speculative)

Fixes #439.

In bash, the suggestions will simply be the available serial numbers. In more capable shells like zsh or fish, the suggestions will be annotated with the same device descriptions as shown by ykman list.

Example: 2021-07-23-20:47:57_1390x472

Marking this as a draft since this would bump the Click dependency to minimum version 8, and we might not want to require that. Version 7 also supports a variant of this, but without the "suggestion help" feature of the more capable shells.

emlun avatar Jul 23 '21 19:07 emlun

Looks interesting. I think we could fairly easily degrade on older versions of Click rather than require Click 8.

The thing that concerns me about this is that enumerating attached YubiKeys with serial numbers is a pretty slow/heavy operation, which also interrupts anything those YubiKeys are currently doing (eg. killing scdaemon, etc.). I'm a bit worried that this would be unexpected to users.

dainnilsson avatar Aug 16 '21 07:08 dainnilsson

Yeah, good point. I experimented a bit more with an explicit opt-in step... try it out. :slightly_smiling_face: I haven't tried it in zsh or fish, but it works pretty well in bash at least. But in the end this might be too much complexity for too little gain.

emlun avatar Aug 17 '21 17:08 emlun

Came to this repo in hopes that bash completion had been implemented, including commands, subcommands, and device numbers. Probably the Click 8 dependency isn't an important consideration 2 years on.

kurtmckee avatar Nov 13 '23 15:11 kurtmckee

Right you are, the dependency in pyproject.toml on main now already specifies 8 as the minimum Click version. So perhaps it's time to revisit this @dainnilsson?

emlun avatar Nov 13 '23 17:11 emlun

The main issue with this wasn't the Click 8 requirement, it was the operation requiring interrupting several attached devices and being slow. Unless that changes this needs to be locked away behind a secret flag, and I don't really see it as very useful in that case. I could probably be convinced otherwise though.

dainnilsson avatar Nov 14 '23 08:11 dainnilsson

Yes, that is still the case, and the feature as implemented is disabled unless the environment variable _YKMAN_EXPERIMENTAL_COMPLETE_DEVICE exists. Sounds like at least @kurtmckee would still use it? I would too, for what it's worth.

emlun avatar Nov 14 '23 12:11 emlun

After some internal discussion I've pushed a new commit with an alternative approach:

Any time devices are enumerated as part of standard usage (most ykman commands will do this) the list of devices is cached in an LRU cache, for use with tab-completion later on. This means that the devices that will complete aren't necessarily present at the moment, but have been "recently" present. The benefit of this is that we never interrupt devices specifically for tab completing purposes, and that it is much faster to actually invoke tab completion. One exception is when running a command without specifying a serial when multiple devices are attached: Here I added an explicit update to the cache as it is very likely the user will immediately try to re-run the command with a serial specified.

The limit on the cache is currently set to 3 devices, which is probably too low, to make testing the behavior a bit easier.

dainnilsson avatar Jan 15 '24 15:01 dainnilsson