canonical / lxd

Powerful system container and virtual machine manager
https://canonical.com/lxd
GNU Affero General Public License v3.0
4.32k stars 925 forks source link

Shell completion improvement suggestions #14098

Open simondeziel opened 1 week ago

simondeziel commented 1 week ago

lxc config get <tab> should suggest:

lxc config get core.<tab> (. is the distinguishing factor) should suggest:

lxc config get foo: core.<tab> (. is the distinguishing factor) should suggest:

lxc config get foo:<tab> (: is the distinguishing factor) should suggest:

lxc config get foo:bar <tab> should suggest:

tomponline commented 1 week ago

@simondeziel thanks!

What about also:

lxc config get foo:core.<tab>

Remote server's options starting with core.

simondeziel commented 1 week ago

What about also:

Good point, added to the issue description! Thanks

kadinsayani commented 1 week ago

Currently, lxc config get <instance> will return all config options (even those which aren't set).

These are the two cases in question:

lxc config get core.<tab> (. is the distinguishing factor) should suggest:

  • local: server's options starting with core.

lxc config get foo:core.<tab> (. is the distinguishing factor) should suggest:

  • foo: server's options starting with core.

With the current behaviour, regardless of the remote, all server options would be suggested. Do we want to change this behaviour to only suggest options which are set? Personally, I find it convenient to use commands like lxc config get <some_config_option> to see if something is already set, or set by default. Happy to take suggestions on this 😄

kadinsayani commented 1 week ago

Also, completion.go is currently undocumented. Would you like me to use this time to also add go doc comments?

simondeziel commented 1 week ago

Currently, lxc config get <instance> will return all config options (even those which aren't set).

That's not what I see with latest/edge:

$ snap list lxd
Name  Version      Rev    Tracking     Publisher   Notes
lxd   git-44a52ea  30256  latest/edge  canonical✓  -

That said, the previous completion script was completing server config options only (not instance) when doing lxc config get <tab>.

With the current behaviour, regardless of the remote, all server options would be suggested. Do we want to change this behaviour to only suggest options which are set? Personally, I find it convenient to use commands like lxc config get <some_config_option> to see if something is already set, or set by default. Happy to take suggestions on this :)

I'm not seeing the same current behaviour as you but aside from that, when discussing with Tom, we concluded that listing all the possible server option would clutter the lxc config get <tab> suggestions and it'd be confusing to have instance names show up in there as well.

In any case, the suggested completion should include all config options irrespective of their value being set or not. The general idea is to make them more easily discoverable and save folks a (slow) trip to the online doc.

kadinsayani commented 1 week ago

Sorry, I should have been clearer. The behaviour I'm referring to can be seen when running lxc config get foo <tab> where foo is the instance.

kadinsayani commented 1 week ago

In any case, the suggested completion should include all config options irrespective of their value being set or not. The general idea is to make them more easily discoverable and save folks a (slow) trip to the online doc.

I think we're on the same page here 😄