From 75bf1a1525e860418b617b6440255fddd2eed205 Mon Sep 17 00:00:00 2001 From: Illia Volochii Date: Thu, 26 Apr 2018 21:10:44 +0300 Subject: [PATCH 1/4] Fix providing multiple values via environment variables --- README.md | 16 ++++++++++++++++ parse.go | 19 ++++++++++++++++++- parse_test.go | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index e8b62a4..362f455 100644 --- a/README.md +++ b/README.md @@ -94,6 +94,22 @@ $ NUM_WORKERS=4 ./example Workers: 4 ``` +You should use a JSON array of strings (value will be converted if +necessary) in the case of multiple values: + +```go +var args struct { + Workers []int `arg:"env"` +} +arg.MustParse(&args) +fmt.Println("Workers:", args.Workers) +``` + +``` +$ WORKERS='["1", "99"]' ./example +Workers: [1 99] +``` + ### Usage strings ```go var args struct { diff --git a/parse.go b/parse.go index 1416223..03b6c07 100644 --- a/parse.go +++ b/parse.go @@ -2,6 +2,7 @@ package arg import ( "encoding" + "encoding/json" "errors" "fmt" "os" @@ -275,7 +276,23 @@ func process(specs []*spec, args []string) error { } if spec.env != "" { if value, found := os.LookupEnv(spec.env); found { - err := scalar.ParseValue(spec.dest, value) + var err error + if spec.multiple { + // expect a JSON array of strings in an environment + // variable in the case of multiple values + var values []string + err = json.Unmarshal([]byte(value), &values) + if err != nil { + return fmt.Errorf( + "error processing environment variable %s (it should be a JSON array of strings):\n%v", + spec.env, + err, + ) + } + err = setSlice(spec.dest, values, !spec.separate) + } else { + err = scalar.ParseValue(spec.dest, value) + } if err != nil { return fmt.Errorf("error processing environment variable %s: %v", spec.env, err) } diff --git a/parse_test.go b/parse_test.go index 1461c02..579d8b7 100644 --- a/parse_test.go +++ b/parse_test.go @@ -580,6 +580,42 @@ func TestEnvironmentVariableRequired(t *testing.T) { assert.Equal(t, "bar", args.Foo) } +func TestEnvironmentVariableSliceArgumentString(t *testing.T) { + var args struct { + Foo []string `arg:"env"` + } + setenv(t, "FOO", "[\"bar\", \"baz\"]") + MustParse(&args) + assert.Equal(t, []string{"bar", "baz"}, args.Foo) +} + +func TestEnvironmentVariableSliceArgumentInteger(t *testing.T) { + var args struct { + Foo []int `arg:"env"` + } + setenv(t, "FOO", "[\"1\", \"99\"]") + MustParse(&args) + assert.Equal(t, []int{1, 99}, args.Foo) +} + +func TestEnvironmentVariableSliceArgumentFloat(t *testing.T) { + var args struct { + Foo []float32 `arg:"env"` + } + setenv(t, "FOO", "[\"1.1\", \"99.9\"]") + MustParse(&args) + assert.Equal(t, []float32{1.1, 99.9}, args.Foo) +} + +func TestEnvironmentVariableSliceArgumentBool(t *testing.T) { + var args struct { + Foo []bool `arg:"env"` + } + setenv(t, "FOO", "[\"true\", \"false\", \"0\", \"1\"]") + MustParse(&args) + assert.Equal(t, []bool{true, false, false, true}, args.Foo) +} + type textUnmarshaler struct { val int } From 488fd7e82ab39ded1347fa792b4243efbc9d4239 Mon Sep 17 00:00:00 2001 From: Illia Volochii Date: Thu, 26 Apr 2018 21:39:41 +0300 Subject: [PATCH 2/4] Add one more test --- parse_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/parse_test.go b/parse_test.go index 579d8b7..f263df5 100644 --- a/parse_test.go +++ b/parse_test.go @@ -616,6 +616,15 @@ func TestEnvironmentVariableSliceArgumentBool(t *testing.T) { assert.Equal(t, []bool{true, false, false, true}, args.Foo) } +func TestEnvironmentVariableSliceArgumentError(t *testing.T) { + var args struct { + Foo []int `arg:"env"` + } + setenv(t, "FOO", "[1, 99]") + err := Parse(&args) + assert.Error(t, err) +} + type textUnmarshaler struct { val int } From fa5fe315f85333e5e7dc0b2147cbe3aae3e4afce Mon Sep 17 00:00:00 2001 From: Illia Volochii Date: Tue, 1 May 2018 12:02:44 +0300 Subject: [PATCH 3/4] Change format from JSON to CSV --- README.md | 5 ++--- parse.go | 16 +++++----------- parse_test.go | 19 +++++-------------- 3 files changed, 12 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 362f455..8980ba1 100644 --- a/README.md +++ b/README.md @@ -94,8 +94,7 @@ $ NUM_WORKERS=4 ./example Workers: 4 ``` -You should use a JSON array of strings (value will be converted if -necessary) in the case of multiple values: +You can provide multiple values using the CSV (RFC 4180) format: ```go var args struct { @@ -106,7 +105,7 @@ fmt.Println("Workers:", args.Workers) ``` ``` -$ WORKERS='["1", "99"]' ./example +$ WORKERS='1,99' ./example Workers: [1 99] ``` diff --git a/parse.go b/parse.go index 03b6c07..1b534fd 100644 --- a/parse.go +++ b/parse.go @@ -2,7 +2,7 @@ package arg import ( "encoding" - "encoding/json" + "encoding/csv" "errors" "fmt" "os" @@ -278,18 +278,12 @@ func process(specs []*spec, args []string) error { if value, found := os.LookupEnv(spec.env); found { var err error if spec.multiple { - // expect a JSON array of strings in an environment + // expect a CSV string in an environment // variable in the case of multiple values - var values []string - err = json.Unmarshal([]byte(value), &values) - if err != nil { - return fmt.Errorf( - "error processing environment variable %s (it should be a JSON array of strings):\n%v", - spec.env, - err, - ) + values, err := csv.NewReader(strings.NewReader(value)).Read() + if err == nil { + err = setSlice(spec.dest, values, !spec.separate) } - err = setSlice(spec.dest, values, !spec.separate) } else { err = scalar.ParseValue(spec.dest, value) } diff --git a/parse_test.go b/parse_test.go index f263df5..56b2b45 100644 --- a/parse_test.go +++ b/parse_test.go @@ -584,16 +584,16 @@ func TestEnvironmentVariableSliceArgumentString(t *testing.T) { var args struct { Foo []string `arg:"env"` } - setenv(t, "FOO", "[\"bar\", \"baz\"]") + setenv(t, "FOO", "bar,\"baz, qux\"") MustParse(&args) - assert.Equal(t, []string{"bar", "baz"}, args.Foo) + assert.Equal(t, []string{"bar", "baz, qux"}, args.Foo) } func TestEnvironmentVariableSliceArgumentInteger(t *testing.T) { var args struct { Foo []int `arg:"env"` } - setenv(t, "FOO", "[\"1\", \"99\"]") + setenv(t, "FOO", "1,99") MustParse(&args) assert.Equal(t, []int{1, 99}, args.Foo) } @@ -602,7 +602,7 @@ func TestEnvironmentVariableSliceArgumentFloat(t *testing.T) { var args struct { Foo []float32 `arg:"env"` } - setenv(t, "FOO", "[\"1.1\", \"99.9\"]") + setenv(t, "FOO", "1.1,99.9") MustParse(&args) assert.Equal(t, []float32{1.1, 99.9}, args.Foo) } @@ -611,20 +611,11 @@ func TestEnvironmentVariableSliceArgumentBool(t *testing.T) { var args struct { Foo []bool `arg:"env"` } - setenv(t, "FOO", "[\"true\", \"false\", \"0\", \"1\"]") + setenv(t, "FOO", "true,false,0,1") MustParse(&args) assert.Equal(t, []bool{true, false, false, true}, args.Foo) } -func TestEnvironmentVariableSliceArgumentError(t *testing.T) { - var args struct { - Foo []int `arg:"env"` - } - setenv(t, "FOO", "[1, 99]") - err := Parse(&args) - assert.Error(t, err) -} - type textUnmarshaler struct { val int } From 89714b6f48fc2660e3fa633ebb7b86c07ce0da01 Mon Sep 17 00:00:00 2001 From: Illia Volochii Date: Mon, 14 May 2018 22:18:05 +0300 Subject: [PATCH 4/4] Fix the problem with errors --- parse.go | 23 ++++++++++++++++------- parse_test.go | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/parse.go b/parse.go index 1b534fd..3c682f5 100644 --- a/parse.go +++ b/parse.go @@ -276,19 +276,28 @@ func process(specs []*spec, args []string) error { } if spec.env != "" { if value, found := os.LookupEnv(spec.env); found { - var err error if spec.multiple { // expect a CSV string in an environment // variable in the case of multiple values values, err := csv.NewReader(strings.NewReader(value)).Read() - if err == nil { - err = setSlice(spec.dest, values, !spec.separate) + if err != nil { + return fmt.Errorf( + "error reading a CSV string from environment variable %s with multiple values: %v", + spec.env, + err, + ) + } + if err = setSlice(spec.dest, values, !spec.separate); err != nil { + return fmt.Errorf( + "error processing environment variable %s with multiple values: %v", + spec.env, + err, + ) } } else { - err = scalar.ParseValue(spec.dest, value) - } - if err != nil { - return fmt.Errorf("error processing environment variable %s: %v", spec.env, err) + if err := scalar.ParseValue(spec.dest, value); err != nil { + return fmt.Errorf("error processing environment variable %s: %v", spec.env, err) + } } spec.wasPresent = true } diff --git a/parse_test.go b/parse_test.go index 56b2b45..0bc97e3 100644 --- a/parse_test.go +++ b/parse_test.go @@ -616,6 +616,24 @@ func TestEnvironmentVariableSliceArgumentBool(t *testing.T) { assert.Equal(t, []bool{true, false, false, true}, args.Foo) } +func TestEnvironmentVariableSliceArgumentWrongCsv(t *testing.T) { + var args struct { + Foo []int `arg:"env"` + } + setenv(t, "FOO", "1,99\"") + err := Parse(&args) + assert.Error(t, err) +} + +func TestEnvironmentVariableSliceArgumentWrongType(t *testing.T) { + var args struct { + Foo []bool `arg:"env"` + } + setenv(t, "FOO", "one,two") + err := Parse(&args) + assert.Error(t, err) +} + type textUnmarshaler struct { val int }