commands: parse arguments with go-opt

Use the argument parsing framework introduced earlier to unify the
parsing of (almost) all command options. Remove custom parsing code and
to avoid extraneous types, add fields with `opt` tags on command structs
that have options and arguments. Commands that take no argument do not
need anything.

Since the command objects now carry data, create a new temporary
instance of them before passing them to opt.ArgsToStruct when executing
a command.

A few of the commands use specific semantics for parsing (:choose), or
are delegating argument parsing to another function (:sort, :search,
:filter). For these commands, simply add a dummy "-" passthrough
argument. Since all commands still have the argument list (after split)
nothing needs to be changed in this area.

There should be no functional change besides the Usage strings and
reported errors which are now generated automatically.

Signed-off-by: Robin Jarry <robin@jarry.cc>
Reviewed-by: Koni Marti <koni.marti@gmail.com>
Tested-by: Moritz Poldrack <moritz@poldrack.dev>
Tested-by: Inwit <inwit@sindominio.net>

8 files changed, 68 insertions, 138 deletions

diff --git a/commands/msgview/close.go b/commands/msgview/close.go
index e0ad6040..32702da9 100644
--- a/commands/msgview/close.go
+++ b/commands/msgview/close.go
@@ -1,8 +1,6 @@
 package msgview
 
 import (
-	"errors"
-
 	"git.sr.ht/~rjarry/aerc/app"
 )
 
@@ -21,9 +19,6 @@ func (Close) Complete(args []string) []string {
 }
 
 func (Close) Execute(args []string) error {
-	if len(args) != 1 {
-		return errors.New("Usage: close")
-	}
 	mv, _ := app.SelectedTabContent().(*app.MessageViewer)
 	app.RemoveTab(mv, true)
 	return nil
diff --git a/commands/msgview/next-part.go b/commands/msgview/next-part.go
index 951842d7..9b1b1fcc 100644
--- a/commands/msgview/next-part.go
+++ b/commands/msgview/next-part.go
@@ -1,13 +1,12 @@
 package msgview
 
 import (
-	"fmt"
-	"strconv"
-
 	"git.sr.ht/~rjarry/aerc/app"
 )
 
-type NextPrevPart struct{}
+type NextPrevPart struct {
+	Offset int `opt:"n" default:"1"`
+}
 
 func init() {
 	register(NextPrevPart{})
@@ -21,22 +20,9 @@ func (NextPrevPart) Complete(args []string) []string {
 	return nil
 }
 
-func (NextPrevPart) Execute(args []string) error {
-	if len(args) > 2 {
-		return nextPrevPartUsage(args[0])
-	}
-	var (
-		n   int = 1
-		err error
-	)
-	if len(args) > 1 {
-		n, err = strconv.Atoi(args[1])
-		if err != nil {
-			return nextPrevPartUsage(args[0])
-		}
-	}
+func (np NextPrevPart) Execute(args []string) error {
 	mv, _ := app.SelectedTabContent().(*app.MessageViewer)
-	for ; n > 0; n-- {
+	for n := 0; n < np.Offset; n++ {
 		if args[0] == "prev-part" {
 			mv.PreviousPart()
 		} else {
@@ -45,7 +31,3 @@ func (NextPrevPart) Execute(args []string) error {
 	}
 	return nil
 }
-
-func nextPrevPartUsage(cmd string) error {
-	return fmt.Errorf("Usage: %s [n]", cmd)
-}
diff --git a/commands/msgview/next.go b/commands/msgview/next.go
index a69cb8ee..d8f046f8 100644
--- a/commands/msgview/next.go
+++ b/commands/msgview/next.go
@@ -3,6 +3,8 @@ package msgview
 import (
 	"errors"
 	"fmt"
+	"strconv"
+	"strings"
 
 	"git.sr.ht/~rjarry/aerc/app"
 	"git.sr.ht/~rjarry/aerc/commands/account"
@@ -11,12 +13,28 @@ import (
 	"git.sr.ht/~rjarry/aerc/worker/types"
 )
 
-type NextPrevMsg struct{}
+type NextPrevMsg struct {
+	Amount  int `opt:"n" default:"1" metavar:"N[%]" action:"ParseAmount"`
+	Percent bool
+}
 
 func init() {
 	register(NextPrevMsg{})
 }
 
+func (np *NextPrevMsg) ParseAmount(arg string) error {
+	if strings.HasSuffix(arg, "%") {
+		np.Percent = true
+		arg = strings.TrimSuffix(arg, "%")
+	}
+	i, err := strconv.ParseInt(arg, 10, 64)
+	if err != nil {
+		return err
+	}
+	np.Amount = int(i)
+	return nil
+}
+
 func (NextPrevMsg) Aliases() []string {
 	return []string{"next", "next-message", "prev", "prev-message"}
 }
@@ -25,11 +43,13 @@ func (NextPrevMsg) Complete(args []string) []string {
 	return nil
 }
 
-func (NextPrevMsg) Execute(args []string) error {
-	n, pct, err := account.ParseNextPrevMessage(args)
+func (np NextPrevMsg) Execute(args []string) error {
+	cmd := account.NextPrevMsg{Amount: np.Amount, Percent: np.Percent}
+	err := cmd.Execute(args)
 	if err != nil {
 		return err
 	}
+
 	mv, _ := app.SelectedTabContent().(*app.MessageViewer)
 	acct := mv.SelectedAccount()
 	if acct == nil {
@@ -39,10 +59,6 @@ func (NextPrevMsg) Execute(args []string) error {
 	if store == nil {
 		return fmt.Errorf("Cannot perform action. No message store set.")
 	}
-	err = account.ExecuteNextPrevMessage(args, acct, pct, n)
-	if err != nil {
-		return err
-	}
 	executeNextPrev := func(nextMsg *models.MessageInfo) {
 		lib.NewMessageStoreView(nextMsg, mv.MessageView().SeenFlagSet(),
 			store, app.CryptoProvider(), app.DecryptKeys,
diff --git a/commands/msgview/open-link.go b/commands/msgview/open-link.go
index 7241fb40..ad2a7cc2 100644
--- a/commands/msgview/open-link.go
+++ b/commands/msgview/open-link.go
@@ -1,7 +1,6 @@
 package msgview
 
 import (
-	"errors"
 	"fmt"
 	"net/url"
 
@@ -11,7 +10,10 @@ import (
 	"git.sr.ht/~rjarry/aerc/log"
 )
 
-type OpenLink struct{}
+type OpenLink struct {
+	Url *url.URL `opt:"url" action:"ParseUrl"`
+	Cmd []string `opt:"..." required:"false"`
+}
 
 func init() {
 	register(OpenLink{})
@@ -31,18 +33,20 @@ func (OpenLink) Complete(args []string) []string {
 	return nil
 }
 
-func (OpenLink) Execute(args []string) error {
-	if len(args) < 2 {
-		return errors.New("Usage: open-link <url> [program [args...]]")
-	}
-	u, err := url.Parse(args[1])
+func (o *OpenLink) ParseUrl(arg string) error {
+	u, err := url.Parse(arg)
 	if err != nil {
 		return err
 	}
-	mime := fmt.Sprintf("x-scheme-handler/%s", u.Scheme)
+	o.Url = u
+	return nil
+}
+
+func (o OpenLink) Execute(args []string) error {
+	mime := fmt.Sprintf("x-scheme-handler/%s", o.Url.Scheme)
 	go func() {
 		defer log.PanicHandler()
-		if err := lib.XDGOpenMime(args[1], mime, args[2:]); err != nil {
+		if err := lib.XDGOpenMime(o.Url.String(), mime, o.Cmd); err != nil {
 			app.PushError("open-link: " + err.Error())
 		}
 	}()
diff --git a/commands/msgview/open.go b/commands/msgview/open.go
index 6c806c7c..bab46bd7 100644
--- a/commands/msgview/open.go
+++ b/commands/msgview/open.go
@@ -7,14 +7,15 @@ import (
 	"os"
 	"path/filepath"
 
-	"git.sr.ht/~sircmpwn/getopt"
-
 	"git.sr.ht/~rjarry/aerc/app"
 	"git.sr.ht/~rjarry/aerc/lib"
 	"git.sr.ht/~rjarry/aerc/log"
 )
 
-type Open struct{}
+type Open struct {
+	Delete bool     `opt:"-d"`
+	Cmd    []string `opt:"..." required:"false"`
+}
 
 func init() {
 	register(Open{})
@@ -33,19 +34,6 @@ func (Open) Complete(args []string) []string {
 }
 
 func (o Open) Execute(args []string) error {
-	opts, optind, err := getopt.Getopts(args, o.Options())
-	if err != nil {
-		return err
-	}
-
-	del := false
-
-	for _, opt := range opts {
-		if opt.Option == 'd' {
-			del = true
-		}
-	}
-
 	mv := app.SelectedTabContent().(*app.MessageViewer)
 	if mv == nil {
 		return errors.New("open only supported selected message parts")
@@ -84,10 +72,10 @@ func (o Open) Execute(args []string) error {
 
 		go func() {
 			defer log.PanicHandler()
-			if del {
+			if o.Delete {
 				defer os.Remove(tmpFile.Name())
 			}
-			err = lib.XDGOpenMime(tmpFile.Name(), mimeType, args[optind:])
+			err = lib.XDGOpenMime(tmpFile.Name(), mimeType, o.Cmd)
 			if err != nil {
 				app.PushError("open: " + err.Error())
 			}
diff --git a/commands/msgview/save.go b/commands/msgview/save.go
index c8e00e4f..ea7599d3 100644
--- a/commands/msgview/save.go
+++ b/commands/msgview/save.go
@@ -9,8 +9,6 @@ import (
 	"strings"
 	"time"
 
-	"git.sr.ht/~sircmpwn/getopt"
-
 	"git.sr.ht/~rjarry/aerc/app"
 	"git.sr.ht/~rjarry/aerc/commands"
 	"git.sr.ht/~rjarry/aerc/config"
@@ -19,7 +17,13 @@ import (
 	"git.sr.ht/~rjarry/aerc/models"
 )
 
-type Save struct{}
+type Save struct {
+	Force          bool   `opt:"-f"`
+	CreateDirs     bool   `opt:"-p"`
+	Attachments    bool   `opt:"-a"`
+	AllAttachments bool   `opt:"-A"`
+	Path           string `opt:"..." required:"false" metavar:"<path>"`
+}
 
 func init() {
 	register(Save{})
@@ -43,77 +47,33 @@ func (s Save) Complete(args []string) []string {
 	return commands.CompletePath(xdg.ExpandHome(path))
 }
 
-type saveParams struct {
-	force          bool
-	createDirs     bool
-	trailingSlash  bool
-	attachments    bool
-	allAttachments bool
-}
-
 func (s Save) Execute(args []string) error {
-	opts, optind, err := getopt.Getopts(args, s.Options())
-	if err != nil {
-		return err
-	}
-
-	var params saveParams
-
-	for _, opt := range opts {
-		switch opt.Option {
-		case 'f':
-			params.force = true
-		case 'p':
-			params.createDirs = true
-		case 'a':
-			params.attachments = true
-		case 'A':
-			params.allAttachments = true
-		}
-	}
-
-	defaultPath := config.General.DefaultSavePath
 	// we either need a path or a defaultPath
-	if defaultPath == "" && len(args) == optind {
-		return errors.New("Usage: :save [-fpa] <path>")
-	}
-
-	// as a convenience we join with spaces, so that the user doesn't need to
-	// quote filenames containing spaces
-	path := strings.Join(args[optind:], " ")
-
-	// needs to be determined prior to calling filepath.Clean / filepath.Join
-	// it gets stripped by Clean.
-	// we auto generate a name if a directory was given
-	if len(path) > 0 {
-		params.trailingSlash = path[len(path)-1] == '/'
-	} else if len(defaultPath) > 0 && len(path) == 0 {
-		// empty path, so we might have a default that ends in a trailingSlash
-		params.trailingSlash = defaultPath[len(defaultPath)-1] == '/'
+	if s.Path == "" && config.General.DefaultSavePath == "" {
+		return errors.New("No default save path in config")
 	}
 
 	// Absolute paths are taken as is so that the user can override the default
 	// if they want to
-	if !isAbsPath(path) {
-		path = filepath.Join(defaultPath, path)
+	if !isAbsPath(s.Path) {
+		s.Path = filepath.Join(config.General.DefaultSavePath, s.Path)
 	}
 
-	path = xdg.ExpandHome(path)
+	s.Path = xdg.ExpandHome(s.Path)
 
 	mv, ok := app.SelectedTabContent().(*app.MessageViewer)
 	if !ok {
 		return fmt.Errorf("SelectedTabContent is not a MessageViewer")
 	}
 
-	if params.attachments || params.allAttachments {
-		parts := mv.AttachmentParts(params.allAttachments)
+	if s.Attachments || s.AllAttachments {
+		parts := mv.AttachmentParts(s.AllAttachments)
 		if len(parts) == 0 {
 			return fmt.Errorf("This message has no attachments")
 		}
-		params.trailingSlash = true
 		names := make(map[string]struct{})
 		for _, pi := range parts {
-			if err := savePart(pi, path, mv, &params, names); err != nil {
+			if err := s.savePart(pi, mv, names); err != nil {
 				return err
 			}
 		}
@@ -121,23 +81,22 @@ func (s Save) Execute(args []string) error {
 	}
 
 	pi := mv.SelectedMessagePart()
-	return savePart(pi, path, mv, &params, make(map[string]struct{}))
+	return s.savePart(pi, mv, make(map[string]struct{}))
 }
 
-func savePart(
+func (s *Save) savePart(
 	pi *app.PartInfo,
-	path string,
 	mv *app.MessageViewer,
-	params *saveParams,
 	names map[string]struct{},
 ) error {
-	if params.trailingSlash || isDirExists(path) {
+	path := s.Path
+	if s.Attachments || s.AllAttachments || isDirExists(path) {
 		filename := generateFilename(pi.Part)
 		path = filepath.Join(path, filename)
 	}
 
 	dir := filepath.Dir(path)
-	if params.createDirs && dir != "" {
+	if s.CreateDirs && dir != "" {
 		err := os.MkdirAll(dir, 0o755)
 		if err != nil {
 			return err
@@ -147,7 +106,7 @@ func savePart(
 	path = getCollisionlessFilename(path, names)
 	names[path] = struct{}{}
 
-	if pathExists(path) && !params.force {
+	if pathExists(path) && !s.Force {
 		return fmt.Errorf("%q already exists and -f not given", path)
 	}
 
diff --git a/commands/msgview/toggle-headers.go b/commands/msgview/toggle-headers.go
index 746692fd..c27307d3 100644
--- a/commands/msgview/toggle-headers.go
+++ b/commands/msgview/toggle-headers.go
@@ -1,8 +1,6 @@
 package msgview
 
 import (
-	"fmt"
-
 	"git.sr.ht/~rjarry/aerc/app"
 )
 
@@ -21,14 +19,7 @@ func (ToggleHeaders) Complete(args []string) []string {
 }
 
 func (ToggleHeaders) Execute(args []string) error {
-	if len(args) > 1 {
-		return toggleHeadersUsage(args[0])
-	}
 	mv, _ := app.SelectedTabContent().(*app.MessageViewer)
 	mv.ToggleHeaders()
 	return nil
 }
-
-func toggleHeadersUsage(cmd string) error {
-	return fmt.Errorf("Usage: %s", cmd)
-}
diff --git a/commands/msgview/toggle-key-passthrough.go b/commands/msgview/toggle-key-passthrough.go
index 1524b420..32735870 100644
--- a/commands/msgview/toggle-key-passthrough.go
+++ b/commands/msgview/toggle-key-passthrough.go
@@ -1,8 +1,6 @@
 package msgview
 
 import (
-	"errors"
-
 	"git.sr.ht/~rjarry/aerc/app"
 	"git.sr.ht/~rjarry/aerc/lib/state"
 )
@@ -22,9 +20,6 @@ func (ToggleKeyPassthrough) Complete(args []string) []string {
 }
 
 func (ToggleKeyPassthrough) Execute(args []string) error {
-	if len(args) != 1 {
-		return errors.New("Usage: toggle-key-passthrough")
-	}
 	mv, _ := app.SelectedTabContent().(*app.MessageViewer)
 	keyPassthroughEnabled := mv.ToggleKeyPassthrough()
 	if acct := mv.SelectedAccount(); acct != nil {