Merge pull request #8321 from dolthub/bh/optional-params

Optional String Argument

go/libraries/utils/argparser/parser.go

@@ -343,6 +343,22 @@ func (ap *ArgParser) Parse(args []string) (*ArgParseResults, error) {
 	return &ArgParseResults{namedArgs, positionalArgs, ap, positionalArgsSeparatorIndex}, nil
 }
 
+func (ap *ArgParser) isOptionOrFlag(s string) bool {
+	if len(s) == 0 {
+		return false
+	} else if s[0] != '-' {
+		return false
+	}
+	s = s[1:]
+
+	if len(s) >= 1 && s[0] == '-' {
+		s = s[1:]
+	}
+
+	_, ok := ap.nameOrAbbrevToOpt[s]
+	return ok
+}
+
 func (ap *ArgParser) parseToken(args []string, index int, positionalArgs []string, namedArgs map[string]string) (newIndex int, newPositionalArgs []string, newNamedArgs map[string]string, err error) {
 	arg := args[index]
 
@@ -387,22 +403,31 @@ func (ap *ArgParser) parseToken(args []string, index int, positionalArgs []strin
 	}
 
 	if value == nil {
-		index++
+		next := index + 1
 		valueStr := ""
-		if index >= len(args) {
+		if next >= len(args) {
 			if opt.OptType != OptionalEmptyValue {
 				return 0, nil, nil, errors.New("error: no value for option `" + opt.Name + "'")
 			}
 		} else {
 			if opt.AllowMultipleOptions {
-				list := getListValues(args[index:])
+				list := getListValues(args[next:])
 				valueStr = strings.Join(list, ",")
-				index += len(list) - 1
+				index += len(list)
 			} else {
-				valueStr = args[index]
+				nextArg := args[next]
+				if opt.OptType == OptionalEmptyValue {
+					if !(nextArg == "--" || ap.isOptionOrFlag(nextArg)) {
+						valueStr = args[next]
+						index = next
+					}
+				} else {
+					valueStr = args[next]
+					index = next
+				}
 			}
+			value = &valueStr
 		}
-		value = &valueStr
 	}
 
 	if opt.Validator != nil {
@@ -413,6 +438,10 @@ func (ap *ArgParser) parseToken(args []string, index int, positionalArgs []strin
 		}
 	}
 
+	if value == nil {
+		value = new(string)
+	}
+
 	namedArgs[opt.Name] = *value
 	return index, positionalArgs, namedArgs, nil
 }

go/libraries/utils/argparser/parser_test.go

@@ -15,13 +15,21 @@
 package argparser
 
 import (
-	"errors"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+func createParserWithOptionalArgs() *ArgParser {
+	ap := NewArgParserWithMaxArgs("test", 16)
+	ap.SupportsFlag("flag", "f", "flag")
+	ap.SupportsString("param", "p", "param", "")
+	ap.SupportsOptionalString("optional", "o", "optional", "")
+
+	return ap
+}
+
 func TestArgParser(t *testing.T) {
 	tests := []struct {
 		ap              *ArgParser
@@ -94,10 +102,52 @@ func TestArgParser(t *testing.T) {
 			[]string{},
 		},
 		{
-			NewArgParserWithMaxArgs("test", 1),
+			createParserWithOptionalArgs(),
 			[]string{"foo", "bar"},
-			errors.New("error: test has too many positional arguments. Expected at most 1, found 2: foo, bar"),
+			nil,
 			map[string]string{},
+			[]string{"foo", "bar"},
+		},
+		{
+			createParserWithOptionalArgs(),
+			[]string{"-o", "-f", "foo", "bar"},
+			nil,
+			map[string]string{"flag": "", "optional": ""},
+			[]string{"foo", "bar"},
+		},
+		{
+			createParserWithOptionalArgs(),
+			[]string{"-o", "optional value", "-f", "foo", "bar"},
+			nil,
+			map[string]string{"flag": "", "optional": "optional value"},
+			[]string{"foo", "bar"},
+		},
+		{
+			createParserWithOptionalArgs(),
+			[]string{"-o", "--", "foo", "bar"},
+			nil,
+			map[string]string{"optional": ""},
+			[]string{"foo", "bar"},
+		},
+		{
+			createParserWithOptionalArgs(),
+			[]string{"-p", "value", "-o"},
+			nil,
+			map[string]string{"param": "value", "optional": ""},
+			[]string{},
+		},
+		{
+			createParserWithOptionalArgs(),
+			[]string{"-p", "value", "-o", "--"},
+			nil,
+			map[string]string{"param": "value", "optional": ""},
+			[]string{},
+		},
+		{
+			createParserWithOptionalArgs(),
+			[]string{"-o", "-p", "value"},
+			nil,
+			map[string]string{"param": "value", "optional": ""},
 			[]string{},
 		},
 	}