Fix '--flag arg' and Allow flags to take optional arguments

[eparis]

Today --flag arg does not work. It just fails to parse. So fix that.

At the same time allow flags to specify that their args are 'optional'. Much as with a bool =true =false are optional. So one could do

var flagvar int
flag.IntVar(&flagvar, "list", 0, "list the number of items")
flag.Lookup("list").NoOptDefVal = 10

In which case --flag and --flag=20 would be valid, but --flag 20 would not work.

[eparis]

I see this as an alternative to #18

Both have the same results, just how and what is specified...

Thoughts?

[eparis]

Since 9294901

long arguments followed by a space do not work

$ ./test --flagname 1235
flag needs an argument: --flagname
Usage of ./test:
      --flagname=1234: help message for flagname

Test program:

import (
    "fmt"
    "os"

    "github.com/spf13/pflag"
)

func main() {
    var flagvar int
    pflag.IntVar(&flagvar, "flagname", 1234, "help message for flagname")
    pflag.Parse()

    fmt.Fprintf(os.Stdout, "val=%v\n", flagvar)
}

So I decided to look at what ls does (and some other coreutils binaries), with 'optional' arguments. Optional arguments can not use the form --flag arg. It must use form --flag=arg.

ls --color is optional:

$ ls --color
$ ls --color=never
$ ls --color never
ls: cannot access never: No such file or directory

Man Page:
       --color[=WHEN]
              colorize the output; WHEN can be 'never', 'auto', or 'always' (the default); more info below

argument to ls --tabsize is not optional

$ ls -T 7
$ ls --tabsize=7
$ ls --tabsize 7
$ ls --tabsize
ls: option '--tabsize' requires an argument
Try 'ls --help' for more information.

Man Page:
       -T, --tabsize=COLS
              assume tab stops at each COLS instead of 8

Given we already do not support --flag arg losing that parse ability by adding optional args is no actual lose.

That means the only place we do lose something is a flag which is both optional and provides a short form. Very few coreutils binaries have both optional and shortform flags. (mktemp, shred, split, tail, uniq) However they also do not allow -s arg for example:

$ tail --follow /tmp/bob

^C
$ tail --follow=name /tmp/bob

^C
$ tail -f name /tmp/bob
tail: cannot open ‘name’ for reading: No such file or directory
==> /tmp/bob <==

^C

We would, I think, want to turn on --flag arg with a patch like the following however....

diff --git a/flag.go b/flag.go
index 9f16733..93222c8 100644
--- a/flag.go
+++ b/flag.go
@@ -530,11 +530,15 @@ func (f *FlagSet) parseLongArg(s string, args []string) (a []string, err error)
        }
        var value string
        if len(split) == 1 {
-               if bv, ok := flag.Value.(boolFlag); !ok || !bv.IsBoolFlag() {
+               if bv, ok := flag.Value.(boolFlag); ok && bv.IsBoolFlag() {
+                       value = "true"
+               } else if len(a) > 0 {
+                       value = a[0]
+                       a = a[1:]
+               } else {
                        err = f.failf("flag needs an argument: %s", s)
                        return
                }
-               value = "true"
        } else {
                value = split[1]
        }

[eparis]

At this point I'm feeling like I want to merge this. It is completely in line with what the ls command does. How much better of an example could we have to follow?

[sgotti]

@eparis I tested your changes (mainly the Add support for '--flag arg') for rkt/rkt#1028. Perhaps just this fix (if the other one needs more approval) should be merged in its own PR?

About the Set default values if no arg given, maybe some documentation explaining the feature (like your examples above or the one provided by #18), and how to detect if a flag was provided or not (looking at the Changed value) will be good (also if this is not related to this patch but this patch makes the difference between provided or not provided stronger).

Thanks!