12876: Simplify legacy option parsing.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --git a/cmd/arvados-client/cmd.go b/cmd/arvados-client/cmd.go
index b96cce368bc4d8b9860a476485beee378c6f9197..73d772a578a71aa70806a78ca6b51e460424c7c3 100644 (file)
--- a/cmd/arvados-client/cmd.go
+++ b/cmd/arvados-client/cmd.go
@@ -5,6 +5,7 @@
 package main
 
 import (
+       "flag"
        "fmt"
        "io"
        "os"
@@ -13,17 +14,18 @@ import (
 
        "git.curoverse.com/arvados.git/lib/cli"
        "git.curoverse.com/arvados.git/lib/cmd"
+       "rsc.io/getopt"
 )
 
 var version = "dev"
 
-var Run = cmd.WithLateSubcommand(cmd.Multi(map[string]cmd.RunFunc{
+var Run = cmd.Multi(map[string]cmd.RunFunc{
        "get":       cli.Get,
        "-e":        cmdVersion,
        "version":   cmdVersion,
        "-version":  cmdVersion,
        "--version": cmdVersion,
-}), []string{"f", "format"}, []string{"n", "dry-run", "v", "verbose", "s", "short"})
+})
 
 func cmdVersion(prog string, args []string, _ io.Reader, stdout, _ io.Writer) int {
        prog = regexp.MustCompile(` -*version$`).ReplaceAllLiteralString(prog, "")
@@ -31,6 +33,19 @@ func cmdVersion(prog string, args []string, _ io.Reader, stdout, _ io.Writer) in
        return 0
 }
 
+func fixLegacyArgs(args []string) []string {
+       flags := getopt.NewFlagSet("", flag.ContinueOnError)
+       flags.Bool("dry-run", false, "dry run")
+       flags.Alias("n", "dry-run")
+       flags.String("format", "json", "output format")
+       flags.Alias("f", "format")
+       flags.Bool("short", false, "short")
+       flags.Alias("s", "short")
+       flags.Bool("verbose", false, "verbose")
+       flags.Alias("v", "verbose")
+       return cmd.SubcommandToFront(args, flags)
+}
+
 func main() {
-       os.Exit(Run(os.Args[0], os.Args[1:], os.Stdin, os.Stdout, os.Stderr))
+       os.Exit(Run(os.Args[0], fixLegacyArgs(os.Args[1:]), os.Stdin, os.Stdout, os.Stderr))
 }

diff --git a/lib/cli/get.go b/lib/cli/get.go
index d3177f32dee9f9facf2a47c52e7402f9cc6ebc5b..baa1df73e7341da026675ff0aef1973f81b7bc2e 100644 (file)
--- a/lib/cli/get.go
+++ b/lib/cli/get.go
@@ -12,6 +12,7 @@ import (
 
        "git.curoverse.com/arvados.git/sdk/go/arvados"
        "github.com/ghodss/yaml"
+       "rsc.io/getopt"
 )
 
 func Get(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int {
@@ -22,17 +23,17 @@ func Get(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer)
                }
        }()
 
-       flags := flag.NewFlagSet(prog, flag.ContinueOnError)
+       flags := getopt.NewFlagSet(prog, flag.ContinueOnError)
        flags.SetOutput(stderr)
 
        format := flags.String("format", "json", "output format (json, yaml, or uuid)")
-       flags.StringVar(format, "f", "json", "output format (json, yaml, or uuid)")
+       flags.Alias("f", "format")
        short := flags.Bool("short", false, "equivalent to --format=uuid")
-       flags.BoolVar(short, "s", false, "equivalent to --format=uuid")
+       flags.Alias("s", "short")
        flags.Bool("dry-run", false, "dry run (ignored, for compatibility)")
-       flags.Bool("n", false, "dry run (ignored, for compatibility)")
+       flags.Alias("n", "dry-run")
        flags.Bool("verbose", false, "verbose (ignored, for compatibility)")
-       flags.Bool("v", false, "verbose (ignored, for compatibility)")
+       flags.Alias("v", "verbose")
        err = flags.Parse(args)
        if err != nil {
                return 2

diff --git a/lib/cmd/cmd.go b/lib/cmd/cmd.go
index 46d996528504546d22da0189720da3c33b8aae27..f33354a24bcc60a90e65dd1ddff9a78d694ac147 100644 (file)
--- a/lib/cmd/cmd.go
+++ b/lib/cmd/cmd.go
@@ -67,37 +67,33 @@ func multiUsage(stderr io.Writer, m map[string]RunFunc) {
        }
 }
 
-// WithLateSubcommand wraps a RunFunc by skipping over some known
-// flags to find a subcommand, and moving that subcommand to the front
-// of the args before calling the wrapped RunFunc. For example:
+type FlagSet interface {
+       Init(string, flag.ErrorHandling)
+       NArg() int
+       Parse([]string) error
+       SetOutput(io.Writer)
+}
+
+// SubcommandToFront silently parses args using flagset, and returns a
+// copy of args with the first non-flag argument moved to the
+// front. If parsing fails or consumes all of args, args is returned
+// unchanged.
 //
-//     // Translate [           --format foo subcommand bar]
-//     //        to [subcommand --format foo            bar]
-//     WithLateSubcommand(fn, []string{"format"}, nil)
-func WithLateSubcommand(run RunFunc, argFlags, boolFlags []string) RunFunc {
-       return func(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int {
-               flags := flag.NewFlagSet("prog", flag.ContinueOnError)
-               for _, arg := range argFlags {
-                       flags.String(arg, "", "")
-               }
-               for _, arg := range boolFlags {
-                       flags.Bool(arg, false, "")
-               }
-               // Ignore errors. We can't report a useful error
-               // message anyway.
-               flags.SetOutput(ioutil.Discard)
-               flags.Usage = func() {}
-               flags.Parse(args)
-               if flags.NArg() > 0 {
-                       // Move the first arg after the recognized
-                       // flags up to the front.
-                       flagargs := len(args) - flags.NArg()
-                       newargs := make([]string, len(args))
-                       newargs[0] = args[flagargs]
-                       copy(newargs[1:flagargs+1], args[:flagargs])
-                       copy(newargs[flagargs+1:], args[flagargs+1:])
-                       args = newargs
-               }
-               return run(prog, args, stdin, stdout, stderr)
+// SubcommandToFront invokes methods on flagset that have side
+// effects, including Parse. In typical usage, flagset will not used
+// for anything else after being passed to SubcommandToFront.
+func SubcommandToFront(args []string, flagset FlagSet) []string {
+       flagset.Init("", flag.ContinueOnError)
+       flagset.SetOutput(ioutil.Discard)
+       if err := flagset.Parse(args); err != nil || flagset.NArg() == 0 {
+               // No subcommand found.
+               return args
        }
+       // Move subcommand to the front.
+       flagargs := len(args) - flagset.NArg()
+       newargs := make([]string, len(args))
+       newargs[0] = args[flagargs]
+       copy(newargs[1:flagargs+1], args[:flagargs])
+       copy(newargs[flagargs+1:], args[flagargs+1:])
+       return newargs
 }

diff --git a/lib/cmd/cmd_test.go b/lib/cmd/cmd_test.go
index 2d9228a30b6746c600cc37ac4a8ce591fcc0d17b..04e98d85c3ec41b08235e6e70dd3bb29337638a4 100644 (file)
--- a/lib/cmd/cmd_test.go
+++ b/lib/cmd/cmd_test.go
@@ -6,6 +6,7 @@ package cmd
 
 import (
        "bytes"
+       "flag"
        "fmt"
        "io"
        "strings"
@@ -51,13 +52,11 @@ func (s *CmdSuite) TestUsage(c *check.C) {
        c.Check(stderr.String(), check.Matches, `(?ms)^unrecognized command "nosuchcommand"\n.*echo.*\n`)
 }
 
-func (s *CmdSuite) TestWithLateSubcommand(c *check.C) {
+func (s *CmdSuite) TestSubcommandToFront(c *check.C) {
        defer cmdtest.LeakCheck(c)()
-       stdout := bytes.NewBuffer(nil)
-       stderr := bytes.NewBuffer(nil)
-       run := WithLateSubcommand(testCmd, []string{"format", "f"}, []string{"n"})
-       exited := run("prog", []string{"--format=yaml", "-n", "-format", "beep", "echo", "hi"}, bytes.NewReader(nil), stdout, stderr)
-       c.Check(exited, check.Equals, 0)
-       c.Check(stdout.String(), check.Equals, "--format=yaml -n -format beep hi\n")
-       c.Check(stderr.String(), check.Equals, "")
+       flags := flag.NewFlagSet("", flag.ContinueOnError)
+       flags.String("format", "json", "")
+       flags.Bool("n", false, "")
+       args := SubcommandToFront([]string{"--format=yaml", "-n", "-format", "beep", "echo", "hi"}, flags)
+       c.Check(args, check.DeepEquals, []string{"echo", "--format=yaml", "-n", "-format", "beep", "hi"})
 }