Add edit command

[junnplus]

Signed-off-by: ye.sijun [email protected]

Fixes: #190

[jandubois]

I think the edit command should restart the instance automatically if it is already running (but don't restart when the editing was aborted). Leaving the instance running with the old config is confusing, as the limactl ls command will report incorrect settings.

You could add --restart=false option if you really want to, but I think it is not needed. People who want to do this can always edit the files under $LIMA_HOME directly; limactl edit should leave the system in a consistent state.

[AkihiroSuda]

Automatic restart may surprise users, so I prefer —restart to be false by default

[jandubois]

Automatic restart may surprise users, so I prefer —restart to be false by default

I fail to see the surprise: you edit the config to change the instance; why would you not expect the instance to reflect the modified settings? To me it is the opposite: When I change the config, e.g. to increase the amount of memory to the instance, I expect those changes to be active when I "save" them.

Think of it as a crude "preferences" or "settings" dialog. It is highly unusual to change settings and not have them take effect when you close that dialog. Some dialogs may have an "Apply" button, so changes don't take effect immediately. But once the dialog is closed, the desired and actual state are reconciled. 😄

How does it make sense to have a running instance that doesn't match its configuration data?

[jandubois]

Also think about the confusion if you don't apply the changes on save, and then forget that you have edited the config: if you edit the instance again a week later, the config file no longer reflects the state of the instance at all, and now you wonder how it all got into this state.

If you want to have a modified config file separate from the running instance, you should make a copy, and edit the copy while it is in stopped state. (I know we don't have a copy/clone command yet).

[junnplus]

I fail to see the surprise: you edit the config to change the instance; why would you not expect the instance to reflect the modified settings? To me it is the opposite: When I change the config, e.g. to increase the amount of memory to the instance, I expect those changes to be active when I "save" them.

I agree with @jandubois . Or can we only allow editing of stopped instances?

[AkihiroSuda]

Not all users expect automatic restart because they might be running some apps with unsaved data. Also, some properties could be updated without restarting (in future).

Or can we only allow editing of stopped instances?

This also SGTM

[mikluko]

IMO there should be a promt about restart. E.g., limactl start asks interactive questions upon vm creation unless --tty=false. Why not use same approch here?

[jandubois]

IMO there should be a promt about restart. E.g., limactl start asks interactive questions upon vm creation unless --tty=false. Why not use same approch here?

I assume you want to ask a question here after the editor has been closed, and not ask if the user actually wants to edit the file.

I would agree with this if we discard the changes when the user decides not to restart the VM, e.g.

$ limactl edit alpine
Waiting for editor...

--- current configuration
+++ new configuration
@@ -9,6 +9,7 @@ images:

 mounts:
 - location: "~"
+  writable: true
 - location: "/tmp/lima"
   writable: true

? Instance "alpine" configuration has been changed  [Use arrows to move, type to filter]
> Apply changes and restart instance
  Edit to make more changes
  Discard changes and exit

If the instance is already stopped, then the first option would simply be `Save changes".

I would still disagree with letting the user keep changes in the file for a running instance without applying them: you either apply or discard.

[jandubois]

I think limactl edit should also validate the changes, and only allow you to apply them, when they pass validation.

[junnplus]

@jandubois I added a patch that only allows editing of non-running instances, what do you think?

[jandubois]

I added a patch that only allows editing of non-running instances, what do you think?

I think this is fine. It addresses my concern that the lima.yaml and running instance are out of sync. Also matches what e.g. VMware Fusion does: it simply disallows you changing certain configurations unless the VM is stopped.

Given that limactl edit is what git developers would call a "porcelain" command, I still think it would be useful to allow editing of running instances, and handle the stopping and restarting automatically, in the way I've outlined above (showing the diff; requiring the user to either restart, discard, or continue to edit).

But I understand that this is outside the scope of your current PR, so will file this as an enhancement request (suggestion, really), so maybe somebody may implement it at a later time.

Still waiting to hear what others think.

[AkihiroSuda]

We should take a lock, but can be another PR

https://github.com/lima-vm/lima/blob/v0.8.1/pkg/lockutil/lockutil.go

[AkihiroSuda]

@jandubois LGTY?

[jandubois]

Thanks, LGTM.

Philosophically it feels weird to me that we don't allow editing of a running instance by stopping it first, but we do ask to start the stopped instance after editing it. But we can debate and change it some other time.

[rxynrg]

Hello everyone, I have a problem with running a command limactl edit <INSTANCE>
I guess you updated the master README without releasing a new version with this functionality?

$ limactl --version
limactl version 0.8.1

$ limactl edit default
FATA[0000] unknown command "edit" for "limactl"

$ limactl help edit
Unknown help topic [`edit`]

[afbjorklund]

You can read the old README, if you want: https://github.com/lima-vm/lima/tree/v0.8.1

[afbjorklund]

Philosophically it feels weird to me that we don't allow editing of a running instance by stopping it first,

I implemented the same in the GUI, to disable the "Edit" button for running instances.

Didn't put up a dialog for starting it, since the "Start" button is right there for clicking...