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 builtin yq support to limactl edit command #1412

Merged
merged 1 commit into from
Mar 25, 2023

Conversation

afbjorklund
Copy link
Member

Also add --tty parameter, similar to the start command, for avoiding user interaction asking whether to start it.

$ limactl edit --tty=false --set ".cpus = 2" default
INFO[0000] Instance "default" configuration edited      
$ limactl edit --tty=false --set ".cpus = 2" default
INFO[0000] Aborting, no changes made to the instance  

Follow-up to:

@AkihiroSuda AkihiroSuda added this to the v0.16.0 milestone Mar 14, 2023
@AkihiroSuda AkihiroSuda added the area/cli limactl CLI user experience label Mar 14, 2023
@jandubois
Copy link
Member

I think limactl edit --set ... should not invoke the editor. I can't think of any situation where I want to set some settings via --set, but also want to use the editor as well; I would simply not bother writing the --set options in the first place.

If you ever wanted to do both (which I believe would be extremely uncommon, but maybe from a script), you could still call the command twice:

limactl edit --tty=false --set ".cpus = 3" default
limactl edit default

Even when --set is specified, we should use the --tty setting to determine if we should ask to start the instance or not.

@afbjorklund
Copy link
Member Author

afbjorklund commented Mar 20, 2023

Hmm, for start we open the editor after --set ? That was the behaviour that was copied here.

The --tty should already affect the start prompt.

@jandubois
Copy link
Member

for start we open the editor after --set ?

No, we don't. We ask if the user wants to edit the YAML file. But I see the point that maybe that shouldn't happen either. But that should be a separate issue.

For edit we always open the editor right away (because that is the point of the command), so it is more jarring that I have to close the editor again even if I didn't want to open it (because I used --set). And if I specify --tty=false to prevent it, then it won't ask me to start the VM, and I have to start it manually as well.

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.

Works as expected now (doesn't invoke editor when using --set), but is not showing the lima.yaml content in the editor anymore.

cmd/limactl/edit.go Outdated Show resolved Hide resolved
Also add --tty parameter, similar to the start command,
for avoiding user interaction asking whether to start it.

Signed-off-by: Anders F Björklund <[email protected]>
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.

Thanks, LGTM

@jandubois jandubois merged commit 2cf4c26 into lima-vm:master Mar 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli limactl CLI user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants