allow optional empty value for string value for arg parsing (#4093)

* arg value should allow optional empty value

* fix godocs

go/cmd/dolt/commands/sqlserver/sqlserver.go

@@ -147,7 +147,7 @@ func (cmd SqlServerCmd) ArgParser() *argparser.ArgParser {
 	ap.SupportsString(persistenceBehaviorFlag, "", "persistence-behavior", fmt.Sprintf("Indicate whether to `load` or `ignore` persisted global variables. Defaults to `%s`.", serverConfig.PersistenceBehavior()))
 	ap.SupportsString(commands.PrivsFilePathFlag, "", "privilege file", "Path to a file to load and store users and grants. Defaults to `$doltcfg-dir/privileges.db`. Will only be created if there is a change to privileges.")
 	ap.SupportsString(allowCleartextPasswordsFlag, "", "allow-cleartext-passwords", "Allows use of cleartext passwords. Defaults to false.")
-	ap.SupportsString(socketFlag, "", "socket file", "Path for the unix socket file. Defaults to '/tmp/mysql.sock'.")
+	ap.SupportsOptionalString(socketFlag, "", "socket file", "Path for the unix socket file. Defaults to '/tmp/mysql.sock'.")
 	return ap
 }
 

go/libraries/utils/argparser/option.go

@@ -24,6 +24,7 @@ type OptionType int
 const (
 	OptionalFlag OptionType = iota
 	OptionalValue
+	OptionalEmptyValue
 )
 
 type ValidationFunc func(string) error

go/libraries/utils/argparser/parser.go

@@ -59,7 +59,7 @@ func NewArgParser() *ArgParser {
 	return &ArgParser{supported, nameOrAbbrevToOpt, nil}
 }
 
-// Adds support for a new argument with the option given. Options must have a unique name and abbreviated name.
+// SupportOption adds support for a new argument with the option given. Options must have a unique name and abbreviated name.
 func (ap *ArgParser) SupportOption(opt *Option) {
 	name := opt.Name
 	abbrev := opt.Abbrev
@@ -87,7 +87,7 @@ func (ap *ArgParser) SupportOption(opt *Option) {
 	}
 }
 
-// Adds support for a new flag (argument with no value). See SupportOpt for details on params.
+// SupportsFlag adds support for a new flag (argument with no value). See SupportOpt for details on params.
 func (ap *ArgParser) SupportsFlag(name, abbrev, desc string) *ArgParser {
 	opt := &Option{name, abbrev, "", OptionalFlag, desc, nil}
 	ap.SupportOption(opt)
@@ -95,7 +95,7 @@ func (ap *ArgParser) SupportsFlag(name, abbrev, desc string) *ArgParser {
 	return ap
 }
 
-// Adds support for a new string argument with the description given. See SupportOpt for details on params.
+// SupportsString adds support for a new string argument with the description given. See SupportOpt for details on params.
 func (ap *ArgParser) SupportsString(name, abbrev, valDesc, desc string) *ArgParser {
 	opt := &Option{name, abbrev, valDesc, OptionalValue, desc, nil}
 	ap.SupportOption(opt)
@@ -103,6 +103,15 @@ func (ap *ArgParser) SupportsString(name, abbrev, valDesc, desc string) *ArgPars
 	return ap
 }
 
+// SupportsOptionalString adds support for a new string argument with the description given and optional empty value.
+func (ap *ArgParser) SupportsOptionalString(name, abbrev, valDesc, desc string) *ArgParser {
+	opt := &Option{name, abbrev, valDesc, OptionalEmptyValue, desc, nil}
+	ap.SupportOption(opt)
+
+	return ap
+}
+
+// SupportsValidatedString adds support for a new string argument with the description given and defined validation function.
 func (ap *ArgParser) SupportsValidatedString(name, abbrev, valDesc, desc string, validator ValidationFunc) *ArgParser {
 	opt := &Option{name, abbrev, valDesc, OptionalValue, desc, validator}
 	ap.SupportOption(opt)
@@ -110,7 +119,7 @@ func (ap *ArgParser) SupportsValidatedString(name, abbrev, valDesc, desc string,
 	return ap
 }
 
-// Adds support for a new uint argument with the description given. See SupportOpt for details on params.
+// SupportsUint adds support for a new uint argument with the description given. See SupportOpt for details on params.
 func (ap *ArgParser) SupportsUint(name, abbrev, valDesc, desc string) *ArgParser {
 	opt := &Option{name, abbrev, valDesc, OptionalValue, desc, isUintStr}
 	ap.SupportOption(opt)
@@ -118,7 +127,7 @@ func (ap *ArgParser) SupportsUint(name, abbrev, valDesc, desc string) *ArgParser
 	return ap
 }
 
-// Adds support for a new int argument with the description given. See SupportOpt for details on params.
+// SupportsInt adds support for a new int argument with the description given. See SupportOpt for details on params.
 func (ap *ArgParser) SupportsInt(name, abbrev, valDesc, desc string) *ArgParser {
 	opt := &Option{name, abbrev, valDesc, OptionalValue, desc, isIntStr}
 	ap.SupportOption(opt)
@@ -184,7 +193,7 @@ func (ap *ArgParser) matchModalOptions(arg string) (matches []*Option, rest stri
 func (ap *ArgParser) sortedValueOptions() []string {
 	vos := make([]string, 0, len(ap.Supported))
 	for s, opt := range ap.NameOrAbbrevToOpt {
-		if opt.OptType == OptionalValue && s != "" {
+		if (opt.OptType == OptionalValue || opt.OptType == OptionalEmptyValue) && s != "" {
 			vos = append(vos, s)
 		}
 	}
@@ -265,11 +274,15 @@ func (ap *ArgParser) Parse(args []string) (*ArgParseResults, error) {
 
 		if value == nil {
 			i++
+			valueStr := ""
 			if i >= len(args) {
-				return nil, errors.New("error: no value for option `" + opt.Name + "'")
+				if opt.OptType != OptionalEmptyValue {
+					return nil, errors.New("error: no value for option `" + opt.Name + "'")
+				}
+			} else {
+				valueStr = args[i]
 			}
 
-			valueStr := args[i]
 			value = &valueStr
 		}
 

integration-tests/bats/sql-server.bats

@@ -1478,6 +1478,23 @@ databases:
     [[ "$output" =~ "database is locked to writes" ]] || false
 }
 
+@test "sql-server: start server with socket option undefined should set default socket path" {
+    skiponwindows "unix socket is not available on Windows"
+    cd repo2
+    DEFAULT_DB="repo2"
+    let PORT="$$ % (65536-1024) + 1024"
+
+    dolt sql-server --port $PORT --user dolt --socket > log.txt 2>&1 &
+    SERVER_PID=$!
+    wait_for_connection $PORT 5000
+
+    server_query repo2 1 "select 1 as col1" "col1\n1"
+
+    run grep '\"/tmp/mysql.sock\"' log.txt
+    [ "$status" -eq 0 ]
+    [ "${#lines[@]}" -eq 1 ]
+}
+
 @test "sql-server: server fails to start up if there is already a file in the socket file path" {
     skiponwindows "unix socket is not available on Windows"
     cd repo2