From 125236694f31f257b12d607690042016f6f60f07 Mon Sep 17 00:00:00 2001
From: Emeric MARTINEAU <11473190+emeric-martineau@users.noreply.github.com>
Date: Fri, 19 Mar 2021 12:09:56 +0100
Subject: [PATCH] cli: improve multiple value management (#8310)

---
 examples/cli.v       |   9 ++--
 vlib/cli/flag.v      | 114 ++++++++++++++++++++++++++-----------------
 vlib/cli/flag_test.v |  41 +++++++++++-----
 3 files changed, 100 insertions(+), 64 deletions(-)

diff --git a/examples/cli.v b/examples/cli.v
index e41aed436..e4fa4e727 100644
--- a/examples/cli.v
+++ b/examples/cli.v
@@ -28,14 +28,13 @@ fn main() {
 	greet_cmd.add_flag(Flag{
 		flag: .int
 		name: 'times'
-		value: ['3']
+		default_value: ['3']
 		description: 'Number of times the message gets printed.'
 	})
 	greet_cmd.add_flag(Flag{
-		flag: .string
-		name: 'fun'
-		multiple: true
-		description: 'Just a dumby flags to show multiple.'
+	 	flag: .string_array
+	 	name: 'fun'
+	 	description: 'Just a dumby flags to show multiple.'
 	})
 	cmd.add_command(greet_cmd)
 	cmd.setup()
diff --git a/vlib/cli/flag.v b/vlib/cli/flag.v
index ad3ded815..be84f6fd1 100644
--- a/vlib/cli/flag.v
+++ b/vlib/cli/flag.v
@@ -5,6 +5,10 @@ pub enum FlagType {
 	int
 	float
 	string
+	// If flag can set multiple time, use array type
+	int_array
+	float_array
+	string_array
 }
 
 // Flag holds information for a command line flag.
@@ -12,22 +16,23 @@ pub enum FlagType {
 // These are typically denoted in the shell by a short form `-f` and/or a long form `--flag`
 pub struct Flag {
 pub mut:
-	flag        FlagType
-	name        string
-	abbrev      string
+	flag FlagType
+	// Name of flag
+	name string
+	// Like short option
+	abbrev string
+	// Desciption of flag
 	description string
 	global      bool
-	required    bool
-	value       []string = []
-	// If allow multiple value.
-	// If bool, multiple has no impact, bool can only set once.
-	// If not multiple, and multiple value set at command args, raise an error.
-	multiple bool
+	// If flag is requierd
+	required bool
+	// Default value if no value provide by command line
+	default_value []string = []
 mut:
 	// Set true if flag found.
 	found bool
-	// Set true at first init value.
-	init bool
+	// Value of flag
+	value []string = []
 }
 
 // get_all_found returns an array of all `Flag`s found in the command parameters
@@ -41,7 +46,10 @@ pub fn (flag Flag) get_bool() ?bool {
 	if flag.flag != .bool {
 		return error('$flag.name: Invalid flag type `$flag.flag`, expected `bool`')
 	}
-	return flag.value.len > 0 && flag.value[0] == 'true'
+
+	val := flag.get_value_or_default_value()
+
+	return val.len > 0 && val[0] == 'true'
 }
 
 // get_bool returns `true` if the flag specified in `name` is set.
@@ -58,30 +66,34 @@ pub fn (flag Flag) get_int() ?int {
 		return error('$flag.name: Invalid flag type `$flag.flag`, expected `int`')
 	}
 
-	if flag.value.len == 0 {
+	val := flag.get_value_or_default_value()
+
+	if val.len == 0 {
 		return 0
 	} else {
-		return flag.value[0].int()
+		return val[0].int()
 	}
 }
 
 // get_ints returns the array of `int` value argument of the flag specified in `name`.
 // get_ints returns an error if the `FlagType` is not integer.
 pub fn (flag Flag) get_ints() ?[]int {
-	if flag.flag != .int {
-		return error('$flag.name: Invalid flag type `$flag.flag`, expected `int`')
+	if flag.flag != .int_array {
+		return error('$flag.name: Invalid flag type `$flag.flag`, expected `int_array`')
 	}
 
-	if flag.value.len == 0 {
+	val := flag.get_value_or_default_value()
+
+	if val.len == 0 {
 		return []int{}
 	} else {
-		mut val := []int{}
+		mut values := []int{}
 
-		for f in flag.value {
-			val << f.int()
+		for f in val {
+			values << f.int()
 		}
 
-		return val
+		return values
 	}
 }
 
@@ -106,30 +118,34 @@ pub fn (flag Flag) get_float() ?f64 {
 		return error('$flag.name: Invalid flag type `$flag.flag`, expected `float`')
 	}
 
-	if flag.value.len == 0 {
+	val := flag.get_value_or_default_value()
+
+	if val.len == 0 {
 		return 0.0
 	} else {
-		return flag.value[0].f64()
+		return val[0].f64()
 	}
 }
 
 // get_floats returns the `f64` value argument of the flag.
 // get_floats returns an error if the `FlagType` is not floating point.
 pub fn (flag Flag) get_floats() ?[]f64 {
-	if flag.flag != .float {
-		return error('$flag.name: Invalid flag type `$flag.flag`, expected `float`')
+	if flag.flag != .float_array {
+		return error('$flag.name: Invalid flag type `$flag.flag`, expected `float_array`')
 	}
 
-	if flag.value.len == 0 {
+	val := flag.get_value_or_default_value()
+
+	if val.len == 0 {
 		return []f64{}
 	} else {
-		mut val := []f64{}
+		mut values := []f64{}
 
-		for f in flag.value {
-			val << f.f64()
+		for f in val {
+			values << f.f64()
 		}
 
-		return val
+		return values
 	}
 }
 
@@ -154,24 +170,28 @@ pub fn (flag Flag) get_string() ?string {
 		return error('$flag.name: Invalid flag type `$flag.flag`, expected `string`')
 	}
 
-	if flag.value.len == 0 {
+	val := flag.get_value_or_default_value()
+
+	if val.len == 0 {
 		return ''
 	} else {
-		return flag.value[0]
+		return val[0]
 	}
 }
 
 // get_strings returns the array of `string` value argument of the flag.
 // get_strings returns an error if the `FlagType` is not string.
 pub fn (flag Flag) get_strings() ?[]string {
-	if flag.flag != .string {
-		return error('$flag.name: Invalid flag type `$flag.flag`, expected `string`')
+	if flag.flag != .string_array {
+		return error('$flag.name: Invalid flag type `$flag.flag`, expected `string_array`')
 	}
 
-	if flag.value.len == 0 {
+	val := flag.get_value_or_default_value()
+
+	if val.len == 0 {
 		return []string{}
 	} else {
-		return flag.value
+		return val
 	}
 }
 
@@ -193,20 +213,12 @@ pub fn (flags []Flag) get_strings(name string) ?[]string {
 // an array of arguments with all consumed elements removed.
 fn (mut flag Flag) parse(args []string, with_abbrev bool) ?[]string {
 	if flag.matches(args, with_abbrev) {
-		// TODO
-		// Si pas multiple generer une erreur
-		// Permettre de récupérer plusieurs valeur
-		if flag.init == false {
-			flag.init = true
-			// Clear defaut value if set
-			flag.value = []
-		}
-
 		if flag.flag == .bool {
 			new_args := flag.parse_bool(args) ?
 			return new_args
 		} else {
-			if flag.value.len > 0 && !flag.multiple {
+			if flag.value.len > 0 && flag.flag != .int_array && flag.flag != .float_array
+				&& flag.flag != .string_array {
 				return error('The argument `$flag.name` accept only one value!')
 			}
 
@@ -286,3 +298,13 @@ fn (flags []Flag) have_abbrev() bool {
 	}
 	return have_abbrev
 }
+
+// Check if value is set by command line option. If not, return default value.
+fn (flag Flag) get_value_or_default_value() []string {
+	if flag.value.len == 0 && flag.default_value.len > 0 {
+		// If default value is set and no value provide, use default value.
+		return flag.default_value
+	} else {
+		return flag.value
+	}
+}
diff --git a/vlib/cli/flag_test.v b/vlib/cli/flag_test.v
index 7fb9a3f68..a40a7c07a 100644
--- a/vlib/cli/flag_test.v
+++ b/vlib/cli/flag_test.v
@@ -18,9 +18,8 @@ fn test_if_string_flag_parses() {
 	assert value == 'value2'
 
 	flag = cli.Flag{
-		flag: .string
+		flag: .string_array
 		name: 'flag'
-		multiple: true
 	}
 	flag.parse(['-flag=value1'], false) or { panic(err) }
 	flag.parse(['-flag=value2'], false) or { panic(err) }
@@ -29,9 +28,8 @@ fn test_if_string_flag_parses() {
 
 	flags := [
 		cli.Flag{
-			flag: .string
+			flag: .string_array
 			name: 'flag'
-			multiple: true
 			value: ['a', 'b', 'c']
 		},
 		cli.Flag{
@@ -88,9 +86,8 @@ fn test_if_int_flag_parses() {
 	assert value == 45
 
 	flag = cli.Flag{
-		flag: .int
+		flag: .int_array
 		name: 'flag'
-		multiple: true
 	}
 
 	flag.parse(['-flag=42'], false) or { panic(err) }
@@ -100,9 +97,8 @@ fn test_if_int_flag_parses() {
 
 	flags := [
 		cli.Flag{
-			flag: .int
+			flag: .int_array
 			name: 'flag'
-			multiple: true
 			value: ['1', '2', '3']
 		},
 		cli.Flag{
@@ -131,14 +127,12 @@ fn test_if_float_flag_parses() {
 	}
 
 	flag.parse(['-flag=3.14159'], false) or { panic(err) }
-	assert flag.value[0].f64() == 3.14159
 	value = flag.get_float() or { panic(err) }
 	assert value == 3.14159
 
 	flag = cli.Flag{
-		flag: .float
+		flag: .float_array
 		name: 'flag'
-		multiple: true
 	}
 
 	flag.parse(['-flag=3.1'], false) or { panic(err) }
@@ -148,9 +142,8 @@ fn test_if_float_flag_parses() {
 
 	flags := [
 		cli.Flag{
-			flag: .float
+			flag: .float_array
 			name: 'flag'
-			multiple: true
 			value: ['1.1', '2.2', '3.3']
 		},
 		cli.Flag{
@@ -199,3 +192,25 @@ fn test_if_multiple_value_on_single_value() {
 		assert true
 	}
 }
+
+fn test_default_value() {
+	mut flag := cli.Flag{
+		flag: .float
+		name: 'flag'
+		default_value: ['1.234']
+	}
+
+	flag.parse(['-flag', '3.14158'], false) or { panic(err) }
+	mut value := flag.get_float() or { panic(err) }
+	assert value == 3.14158
+
+	flag = cli.Flag{
+		flag: .float
+		name: 'flag'
+		default_value: ['1.234']
+	}
+
+	flag.parse([''], false) or { panic(err) }
+	value = flag.get_float() or { panic(err) }
+	assert value == 1.234
+}
-- 
2.30.2