Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add LXC/LXD completions. #3554

Merged
merged 1 commit into from Nov 18, 2016
Merged

Add LXC/LXD completions. #3554

merged 1 commit into from Nov 18, 2016

Conversation

@haarts
Copy link
Contributor

haarts commented Nov 15, 2016

Description

This adds (part of) the completions for LXD.
At the moment this covers only the most basic use cases.
Texts are taken from lxc help command.

TODOs:

  • [x ] Changes to fish usage are reflected in user documenation/manpages. not required here
  • [x ] Tests have been added for regressions fixed. not required here
end

function __fish_lxc_list_containers
lxc list -c n | grep '| \w' | sed -e 's/\(|\| \)*//g'

This comment has been minimized.

@faho

faho Nov 15, 2016 Member

We prefer using our string tool for completions - string match -r '\| \w+' | string replace -r '\| ' ''?

This comment has been minimized.

@haarts

haarts Nov 16, 2016 Author Contributor

Splendid! Fixed.

complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments monitor --description 'Monitor activity on the LXD server.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments move --description 'Move containers within or in between lxd instances.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments network --description 'Manage networks.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments pauze --description 'Changes state of one or more containers to pause.'

This comment has been minimized.

@faho

faho Nov 15, 2016 Member

Typo: I assume it's "pause"? Also "changes state to X" sounds a bit weird - why not just "pauses container" and similar for the other states.

This comment has been minimized.

@haarts

haarts Nov 16, 2016 Author Contributor

Typo indeed.

complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments delete --description 'Delete containers or container snapshots.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments exec --description 'Execute the specified command in a container.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments file --description 'Manage files on a container.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments finger --description 'Fingers the LXD instance to check if it is up and working.'

This comment has been minimized.

@faho

faho Nov 15, 2016 Member

"Check if the LXD instance is up"?

complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments exec --description 'Execute the specified command in a container.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments file --description 'Manage files on a container.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments finger --description 'Fingers the LXD instance to check if it is up and working.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments help --description 'Usage: lxc [subcommand] [options]'

This comment has been minimized.

@faho

faho Nov 15, 2016 Member

"Print help"?

This comment has been minimized.

@haarts

haarts Nov 16, 2016 Author Contributor

This is the only one I had to depart from the lxc help <command> approach. I'll change it.


complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments config --description 'Manage configuration.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments copy --description 'Copy containers within or in between lxd instances.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments delete --description 'Delete containers or container snapshots.'

This comment has been minimized.

@faho

faho Nov 15, 2016 Member

Would just "containers or snapshots" be okay here?

Descriptions should be as short as possible to allow more columns to be used.

This comment has been minimized.

@haarts

haarts Nov 16, 2016 Author Contributor

I agree with you that that text would be better. However, I took these texts directly from the lxc help <command> output. Leaving it would mean the original output is the One And Only Truth. I kinda like that too.
I'm a little on the fence here.

complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments help --description 'Usage: lxc [subcommand] [options]'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments image --description 'Manipulate container images.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments info --description 'List information on LXD servers and containers.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments init --description 'Initialize a container from a particular image.'

This comment has been minimized.

@faho

faho Nov 15, 2016 Member

The "particular" seems redundant.

complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments publish --description 'Publish containers as images.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments remote --description 'Manage remote LXD servers.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments restart --description 'Changes state of one or more containers to restart.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments restore --description 'Set the current state of a resource back to a snapshot.'

This comment has been minimized.

@faho

faho Nov 15, 2016 Member

What's "resource" here? Something different from a container? A superset?

At the moment this covers only the most basic use cases.
Texts are taken from `lxc help` command.
@haarts haarts force-pushed the haarts:add-lxc-completions branch from 5a6e793 to 5b50744 Nov 16, 2016
@faho
faho approved these changes Nov 17, 2016
@faho
Copy link
Member

faho commented Nov 17, 2016

@haarts: I'll leave the wording decisions up to you. Let me know when you've decided.

@faho faho added this to the fish 2.5.0 milestone Nov 17, 2016
@haarts
Copy link
Contributor Author

haarts commented Nov 18, 2016

@faho I'd like to echo the wording of LXD. But I agree with your suggestion. Lemme try to fix that upstream. In the meanwhile I think we should merge this. When upstream changes I'll create an other PR.

@haarts
Copy link
Contributor Author

haarts commented Nov 18, 2016

Upstream issue: lxc/lxd#2627

@faho faho merged commit 36d4283 into fish-shell:master Nov 18, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@faho
Copy link
Member

faho commented Nov 18, 2016

Merged, let's see what upstream thinks.

@haarts haarts mentioned this pull request Nov 18, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.