feat(args)!: refactor args (breaking!) (#15)

Now all subcommands accept VMID as the first unnamed arg.

cmd/clone/clone.go

@@ -17,32 +17,12 @@ var Command = &cli.Command{
 	Usage:  "Clone a virtual machine.",
 	Action: cloneVm,
 	Flags: []cli.Flag{
-		&cli.Uint64Flag{
-			Name:     "vmid",
-			Usage:    "`VMID` to clone from",
-			Required: true,
-			Aliases:  []string{"v"},
-			Action: func(c *cli.Context, vmid uint64) error {
-				if vmid < 100 || vmid > 999999999 {
-					return fmt.Errorf("vmid %d out of range", vmid)
-				}
-				return nil
-			},
-		},
 		&cli.Uint64Flag{
 			Name:        "newid",
 			Usage:       "`VMID` for the clone",
 			Required:    false,
 			Aliases:     []string{"n"},
 			DefaultText: "next available",
-			Action: func(c *cli.Context, vmid uint64) error {
-				if vmid < 100 || vmid > 999999999 {
-					if vmid != 0 {
-						return fmt.Errorf("vmid %d out of range", vmid)
-					}
-				}
-				return nil
-			},
 		},
 
 		&cli.Uint64Flag{
@@ -108,16 +88,7 @@ func bool2uint8(b bool) uint8 {
 
 // Clones a Proxmox VM as specified by the `from` arg and `params` struct
 func cloneVm(c *cli.Context) error {
-	cloneOptions := proxmox.VirtualMachineCloneOptions{
-		NewID:    int(c.Uint64("newid")),
-		BWLimit:  c.Uint64("bwlimit"),
-		Full:     bool2uint8(c.Bool("full")),
-		Name:     c.String("name"),
-		Pool:     c.String("pool"),
-		SnapName: c.String("snapname"),
-		Storage:  c.String("storage"),
-		Target:   c.String("target"),
-	}
+	newId := c.Uint64("newid")
 
 	client := util.InstantiateClient(
 		util.GetPveUrl(c),
@@ -127,11 +98,29 @@ func cloneVm(c *cli.Context) error {
 			Realm:    c.String("pverealm"),
 		},
 	)
-	vmid, newId := c.Uint64("vmid"), c.Uint64("newid")
+
+	vmid, err := util.GetVmidArg(c.Args().Slice())
+
 	vm, err := util.GetVirtualMachineByVMID(c.Context, vmid, client)
 	if err != nil {
 		return err
 	}
+	if newId == 0 { // if newId isn't set
+		newIdT, err := util.GetVmidArg(c.Args().Tail())
+		if err == nil {
+			newId = uint64(newIdT)
+		}
+	}
+	cloneOptions := proxmox.VirtualMachineCloneOptions{
+		NewID:    int(newId),
+		BWLimit:  c.Uint64("bwlimit"),
+		Full:     bool2uint8(c.Bool("full")),
+		Name:     c.String("name"),
+		Pool:     c.String("pool"),
+		SnapName: c.String("snapname"),
+		Storage:  c.String("storage"),
+		Target:   c.String("target"),
+	}
 
 	if newId != 0 { // if we're manually assigning the target VMID
 		vmWithSameId, _ := util.GetVirtualMachineByVMID(
@@ -147,20 +136,21 @@ func cloneVm(c *cli.Context) error {
 				if err != nil {
 					return err
 				}
-				logrus.Info("Overwrite requested.")
-				logrus.Warnf("Destroying VM %#v.\n%#v\n", vmWithSameId, task)
+				logrus.Info("overwrite requested\n")
+				logrus.Warnf("destroying VM %d (%s)...\n", vmWithSameId.VMID, vmWithSameId.Name)
+				logrus.Debugf("task: %s\n", task.UPID)
 
 				// err = tasks.WaitTask(c.Context, task, tasks.WithSpinner())
 				err = taskstatus.WaitForCliTask(c, &task)
 				if err != nil {
 					return err
 				}
 
-				logrus.Infof("task: %#v\n", task)
+				logrus.Debugf("task: %s\n", task.UPID)
 			case false:
+				logrus.Tracef("%#v\n", vmWithSameId)
 				return fmt.Errorf(
-					"Use --overwrite if necessary.\n"+
-						"%#v\n", vmWithSameId,
+					"Use --overwrite if necessary.\n",
 				)
 			}
 
@@ -180,7 +170,8 @@ func cloneVm(c *cli.Context) error {
 		newVmid = cloneOptions.NewID
 	}
 
-	logrus.Infof("clone requested! new id: %d.\n%#v\n", newVmid, task)
+	logrus.Infof("clone requested! new id: %d.\n", newVmid)
+	logrus.Tracef("%#v\n", task)
 	if c.Bool("wait") {
 		err = taskstatus.WaitForCliTask(c, task)
 		if err != nil {

cmd/config/config.go

@@ -3,7 +3,6 @@ package config
 import (
 	"encoding/json"
 	"fmt"
-	"strconv"
 	"strings"
 
 	"github.com/b-/gomox/util"
@@ -29,7 +28,8 @@ func pveVersion(c *cli.Context) error {
 			Realm:    c.String("pverealm"),
 		},
 	)
-	vmid, err := strconv.Atoi(c.Args().First())
+
+	vmid, err := util.GetVmidArg(c.Args().Slice())
 	if err != nil {
 		return err
 	}

cmd/destroy/destroy.go

@@ -14,18 +14,6 @@ var Command = &cli.Command{
 	Usage:  "Delete a virtual machine",
 	Action: destroyVmCmd,
 	Flags: []cli.Flag{
-		&cli.Uint64Flag{
-			Name:     "vmid",
-			Usage:    "`VMID` to delete",
-			Required: true,
-			Aliases:  []string{"v"},
-			Action: func(c *cli.Context, vmid uint64) error {
-				if vmid < 100 || vmid > 999999999 {
-					return fmt.Errorf("VM vmid %d out of range", vmid)
-				}
-				return nil
-			},
-		},
 		&cli.BoolFlag{
 			Name:  "force",
 			Usage: "If the VM is not stopped, stop before attempting removal.",
@@ -47,7 +35,11 @@ func destroyVmCmd(c *cli.Context) error {
 			Realm:    c.String("pverealm"),
 		},
 	)
-	vmid := c.Uint64("vmid")
+
+	vmid, err := util.GetVmidArg(c.Args().Slice())
+	if err != nil {
+		return err
+	}
 
 	vm, err := util.GetVirtualMachineByVMID(c.Context, vmid, client)
 	if err != nil {

cmd/start/start.go

@@ -10,22 +10,11 @@ import (
 )
 
 var Command = &cli.Command{
-	Name:   "start",
-	Usage:  "start a virtual machine",
-	Action: startVm,
+	Name:      "start",
+	Usage:     "start a virtual machine",
+	UsageText: "gomox start <VMID>",
+	Action:    startVm,
 	Flags: []cli.Flag{
-		&cli.Uint64Flag{
-			Name:     "vmid",
-			Usage:    "`VMID` to start",
-			Required: true,
-			Aliases:  []string{"v"},
-			Action: func(c *cli.Context, vmid uint64) error {
-				if vmid < 100 || vmid > 999999999 {
-					return fmt.Errorf("VM vmid %d out of range", vmid)
-				}
-				return nil
-			},
-		},
 		&cli.BoolFlag{
 			Name:  "idempotent",
 			Usage: "Don't return error if VM is already in requested state",
@@ -47,7 +36,10 @@ func startVm(c *cli.Context) error {
 		},
 	)
 
-	vmid := c.Uint64("vmid")
+	vmid, err := util.GetVmidArg(c.Args().Slice())
+	if err != nil {
+		return err
+	}
 
 	vm, err := util.GetVirtualMachineByVMID(c.Context, vmid, client)
 	if err != nil {

cmd/stop/stop.go

@@ -10,22 +10,11 @@ import (
 )
 
 var Command = &cli.Command{
-	Name:   "stop",
-	Usage:  "Stop a virtual machine",
-	Action: stopVm,
+	Name:      "stop",
+	Usage:     "Stop a virtual machine",
+	UsageText: "stop <VMID>",
+	Action:    stopVm,
 	Flags: []cli.Flag{
-		&cli.Uint64Flag{
-			Name:     "vmid",
-			Usage:    "`VMID` to stop",
-			Required: true,
-			Aliases:  []string{"v"},
-			Action: func(c *cli.Context, vmid uint64) error {
-				if vmid < 100 || vmid > 999999999 {
-					return fmt.Errorf("VM vmid %d out of range", vmid)
-				}
-				return nil
-			},
-		},
 		&cli.BoolFlag{
 			Name:  "idempotent",
 			Usage: "Don't return error if VM is already in requested state",
@@ -44,7 +33,10 @@ func stopVm(c *cli.Context) error {
 			Realm:    c.String("pverealm"),
 		},
 	)
-	vmid := c.Uint64("vmid")
+	vmid, err := util.GetVmidArg(c.Args().Slice())
+	if err != nil {
+		return err
+	}
 
 	vm, err := util.GetVirtualMachineByVMID(c.Context, vmid, client)
 	if err != nil {

tasks/tasks.go

@@ -13,6 +13,7 @@ import (
 )
 
 const (
+	NoContent             = "no content" // filter this out from logs
 	DefaultTimeout        = 60 * time.Second
 	DefaultPollDuration   = time.Millisecond * 500
 	DefaultSpinnerCharSet = 9 // Classic Unix quiet |/-\|
@@ -173,7 +174,9 @@ func WaitTask(ctx context.Context, task *proxmox.Task, opts ...WaitOption) (err
 		return err
 	}
 	taskname := taskhead[0]
-	fmt.Println(taskname)
+	if taskname != NoContent {
+		fmt.Println(taskname)
+	}
 	c := &waitConfig{
 		quiet: true, // default to quiet
 		spinnerConfig: spinnerConfig{

util/vmidarger.go

@@ -0,0 +1,31 @@
+package util
+
+import (
+	"fmt"
+	"strconv"
+)
+
+const (
+	MaxVmid = 999999999
+	MinVmid = 100
+)
+
+func VmidOutOfRangeError() error {
+	return fmt.Errorf("please supply a VMID between %d and %d", MinVmid, MaxVmid)
+}
+func CheckVmidRange(vmid uint64) error {
+	if vmid < MinVmid || vmid > MaxVmid {
+		return VmidOutOfRangeError()
+	}
+	return nil
+}
+
+func GetVmidArg(args []string) (uint64, error) {
+	ivmid, err := strconv.Atoi(args[0])
+	err = CheckVmidRange(uint64(ivmid))
+	if err != nil {
+		return 0, VmidOutOfRangeError()
+	}
+	vmid := uint64(ivmid)
+	return vmid, nil
+}