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

make nerdctl.lima completion works #234

Merged
merged 1 commit into from
Sep 16, 2021
Merged

Conversation

robberphex
Copy link
Contributor

  • switch to cobra
  • make nerdctl.lima completion works
  • support double dash --.(limactl shell default -- echo --help can work now)
  • support auto-completion at bash\fish\zsh.
  • fix nerdctl.lima completion doesn't work #233

@AkihiroSuda
Copy link
Member

Thanks, but limactl start [TAB] no longer seems to complete file names

@robberphex
Copy link
Contributor Author

Thanks, but limactl start [TAB] no longer seems to complete file names

I tested at my computer, it works:

$ source <(limactl completion bash)
$ limactl start <tab>
default  new
$ limactl __complete start ''
default
new
:4
Completion ended with directive: ShellCompDirectiveNoFileComp

@jandubois Could you paste the output of limactl __complete start ''?

@AkihiroSuda
Copy link
Member

$ limactl start
default new

This should show file names in addition to instance names.

@jandubois
Copy link
Member

@jandubois Could you paste the output of limactl __complete start ''?

alpine
:4
Completion ended with directive: ShellCompDirectiveNoFileComp

@robberphex robberphex force-pushed the use-cobra branch 4 times, most recently from 6103a70 to 9d701db Compare September 11, 2021 05:02
@robberphex
Copy link
Contributor Author

@jandubois github.com/urfave/cli/v2 and github.com/spf13/cobra are not support completing with given value and file names.

So, in this PR, keep this behavior.

@AkihiroSuda
Copy link
Member

$ limactl validate /dev/null 
Error: failed to load YAML file "/dev/null": field `images` must be set
Usage:
  limactl validate FILE.yaml [FILE.yaml, ...] [flags]

Flags:
  -h, --help   help for validate

Global Flags:
      --debug   debug mode

FATA[0000] failed to load YAML file "/dev/null": field `images` must be set 

Can we delete the CLI usage message like this?

@robberphex
Copy link
Contributor Author

$ limactl validate /dev/null 
Error: failed to load YAML file "/dev/null": field `images` must be set
Usage:
  limactl validate FILE.yaml [FILE.yaml, ...] [flags]

Flags:
  -h, --help   help for validate

Global Flags:
      --debug   debug mode

FATA[0000] failed to load YAML file "/dev/null": field `images` must be set 

Can we delete the CLI usage message like this?

Which line to delete?

@AkihiroSuda
Copy link
Member

Which line to delete?

All lines but the last one.

The output should be like this, as in the current master

$ limactl validate /dev/null 
FATA[0000] failed to load YAML file "/dev/null": field `images` must be set 

@robberphex
Copy link
Contributor Author

robberphex commented Sep 14, 2021

Which line to delete?

All lines but the last one.

The output should be like this, as in the current master

$ limactl validate /dev/null 
FATA[0000] failed to load YAML file "/dev/null": field `images` must be set 

Fix this at commit 80934268.

$ limactl validate /dev/null
FATA[0000] failed to load YAML file "/dev/null": field `images` must be set

@robberphex robberphex force-pushed the use-cobra branch 2 times, most recently from 72d1ef4 to 8093426 Compare September 15, 2021 01:21
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@robberphex
Copy link
Contributor Author

@jandubois Hello, is there anything to fix in this PR?

@jandubois
Copy link
Member

@jandubois Hello, is there anything to fix in this PR?

I don't know yet. 😄 I suspect it is good now, but I will do a review tomorrow, and hopefully approve, unless I find anything new. Sorry, I haven't been paying attention during the recent updates.

cmd/limactl/copy.go Outdated Show resolved Hide resolved
cmd/limactl/delete.go Outdated Show resolved Hide resolved
@csh0101
Copy link

csh0101 commented Sep 16, 2021 via email

@jandubois
Copy link
Member

why this message is to me?……

You are watching the lima-vm/lima repository. Maybe you selected "All activity"? Or you pressed the "subscribe" button on issue #234. That's the only explanations I can come up with.

If you don't want further notifications, please unsubscribe from this issue (and change your repo notification settings, if that was the root cause). But since you've replied to the notifications, you are also subscribed to this issue now.

cmd/limactl/hostagent.go Outdated Show resolved Hide resolved
@AkihiroSuda AkihiroSuda modified the milestones: v0.6.4, v0.7.0 Sep 16, 2021
@csh0101
Copy link

csh0101 commented Sep 16, 2021 via email

cmd/limactl/list.go Outdated Show resolved Hide resolved
cmd/limactl/shell.go Outdated Show resolved Hide resolved
@robberphex robberphex force-pushed the use-cobra branch 2 times, most recently from 6f6208a to 137b918 Compare September 16, 2021 06:39
cmd/limactl/shell.go Outdated Show resolved Hide resolved
cmd/limactl/shell.go Outdated Show resolved Hide resolved
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressing the shell error exit is the only remaining issue afaict, so pre-emptively approving before I go to sleep... 😸

@robberphex robberphex force-pushed the use-cobra branch 2 times, most recently from 25b974c to 50109f9 Compare September 16, 2021 07:47
@AkihiroSuda
Copy link
Member

limactl start doesn't seem to work

$ limactl start
FATA[0000] accepts 1 arg(s), received 0  

@AkihiroSuda
Copy link
Member

Could you squash commits?

@jandubois
Copy link
Member

limactl start doesn't seem to work

I'm sorry, I think that is my fault: I asked for Args: cobra.ExactArgs(1) when it should have been Args: cobra.MaximumNArgs(1). Sorry about that! 😞

I believe this, and the squashing of commits are the only outstanding issues now.

Signed-off-by: luyanbo <robert.lyb@alibaba-inc.com>
@robberphex
Copy link
Contributor Author

robberphex commented Sep 16, 2021

  • limactl start doesn't seem to work, fixed
  • squash commits, squashed

@AkihiroSuda I think it's time to merge.

@jandubois jandubois merged commit 06e35a6 into lima-vm:master Sep 16, 2021
@jandubois
Copy link
Member

Thank you!

@robberphex robberphex deleted the use-cobra branch September 16, 2021 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nerdctl.lima completion doesn't work
4 participants