Add a String() to Set and Array so it looks better on UI and symetrical with coma input (#4)

* Add a String() to Set so it looks better on UI and symetrical with coma separated input

* fix array case too and show in example

* fix test (or... made it pass)

* actually fix (and add a clearer) test for default value of arrays

* add a bunch of set functions

* switch for fortio.org/sets

Author: ldemailly

dyngeneric.go

@@ -10,6 +10,7 @@ import (
 	"sync/atomic"
 	"time"
 
+	"fortio.org/sets"
 	"golang.org/x/exp/constraints"
 )
 
@@ -50,19 +51,17 @@ func (*DynamicBoolValueTag) IsBoolFlag() bool {
 
 // ---- Generics section ---
 
-type Set[T comparable] map[T]struct{}
-
 // ValidateDynSetMinElements validates that the given Set has at least x elements.
-func ValidateDynSetMinElements[T comparable](count int) func(Set[T]) error {
-	return func(value Set[T]) error {
+func ValidateDynSetMinElements[T comparable](count int) func(sets.Set[T]) error {
+	return func(value sets.Set[T]) error {
 		if len(value) < count {
 			return fmt.Errorf("value set %+v must have at least %v elements", value, count)
 		}
 		return nil
 	}
 }
 
-// ValidateDynSliceMinElements validates that the given Set has at least x elements.
+// ValidateDynSliceMinElements validates that the given array has at least x elements.
 func ValidateDynSliceMinElements[T any](count int) func([]T) error {
 	return func(value []T) error {
 		if len(value) < count {
@@ -75,7 +74,7 @@ func ValidateDynSliceMinElements[T any](count int) func([]T) error {
 // DynValueTypes are the types currently supported by Parse[T] and thus by Dyn[T].
 // DynJSON is special.
 type DynValueTypes interface {
-	bool | time.Duration | float64 | int64 | string | []string | Set[string]
+	bool | time.Duration | float64 | int64 | string | []string | sets.Set[string]
 }
 
 type DynValue[T any] struct {
@@ -119,7 +118,7 @@ func FlagSet[T DynValueTypes](flagSet *flag.FlagSet, name string, dynValue *DynV
 	dynValue.flagSet = flagSet
 	dynValue.flagName = name
 	flagSet.Var(dynValue, name, dynValue.usage)
-	flagSet.Lookup(name).DefValue = fmt.Sprintf("%v", dynValue.av.Load())
+	flagSet.Lookup(name).DefValue = dynValue.String()
 	return dynValue
 }
 
@@ -194,24 +193,15 @@ func parse[T any](input string) (val T, err error) {
 		*v = input
 	case *[]string:
 		*v = CommaStringToSlice(input)
-	case *Set[string]:
-		*v = SetFromSlice(CommaStringToSlice(input))
+	case *sets.Set[string]:
+		*v = sets.FromSlice(CommaStringToSlice(input))
 	default:
 		// JSON Set() and thus Parse() is handled in dynjson.go
 		err = fmt.Errorf("unexpected type %T", val)
 	}
 	return
 }
 
-// SetFromSlice constructs a Set from a slice.
-func SetFromSlice[T comparable](items []T) Set[T] {
-	res := map[T]struct{}{}
-	for _, item := range items {
-		res[item] = struct{}{}
-	}
-	return res
-}
-
 // Set updates the value from a string representation in a thread-safe manner.
 // This operation may return an error if the provided `input` doesn't parse, or the resulting value doesn't pass an
 // optional validator.
@@ -282,7 +272,12 @@ func (d *DynValue[T]) Type() string {
 
 // String returns the canonical string representation of the type.
 func (d *DynValue[T]) String() string {
-	return fmt.Sprintf("%v", d.Get())
+	switch v := any(d.Get()).(type) {
+	case []string:
+		return strings.Join(v, ",")
+	default:
+		return fmt.Sprintf("%v", d.Get())
+	}
 }
 
 // WithValueMutator adds a function that changes the value of a flag as needed.

dyngeneric_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"fortio.org/assert"
+	"fortio.org/sets"
 )
 
 // Additional generic tests, most tests are covered by the old per type tests.
@@ -31,3 +32,41 @@ func TestDflag_NonDynamic(t *testing.T) {
 	assert.True(t, static != nil)
 	assert.False(t, IsFlagDynamic(static))
 }
+
+func TestSetToString(t *testing.T) {
+	s := sets.Set[string]{"z": {}, "a": {}, "c": {}, "b": {}}
+	f := New(s, "test set")
+	assert.Equal(t, "a,b,c,z", s.String())
+	assert.Equal(t, "a,b,c,z", f.Get().String())
+}
+
+func TestArrayToString(t *testing.T) {
+	s := []string{"z", "a", "c", "b"}
+	f := New(s, "test array")
+	Flag("testing123", f)
+	defValue := flag.CommandLine.Lookup("testing123").DefValue
+	// order preserved unlike for sets.Set where we sort
+	str := f.String()
+	assert.Equal(t, "z,a,c,b", str)
+	assert.Equal(t, "z,a,c,b", defValue)
+}
+
+func TestRemoveCommon(t *testing.T) {
+	setA := sets.New("a", "b", "c", "d")
+	setB := sets.New("b", "d", "e", "f", "g")
+	setAA := setA.Clone()
+	setBB := setB.Clone()
+	sets.RemoveCommon(setAA, setBB)
+	assert.Equal(t, "a,c", setAA.String())   // removed
+	assert.Equal(t, "e,f,g", setBB.String()) // added
+	// Swap order to exercise the optimization on length of iteration
+	// also check clone is not modifying the original etc
+	setAA = setB.Clone() // putting B in AA on purpose and vice versa
+	setBB = setA.Clone()
+	sets.RemoveCommon(setAA, setBB)
+	assert.Equal(t, "a,c", setBB.String())
+	assert.Equal(t, "e,f,g", setAA.String())
+	assert.True(t, setBB.Has("c"))
+	setBB.Remove("c")
+	assert.False(t, setBB.Has("c"))
+}

dynstringset.go

@@ -6,20 +6,22 @@ package dflag
 import (
 	"flag"
 	"fmt"
+
+	"fortio.org/sets"
 )
 
 // DynStringSet creates a `Flag` that represents `map[string]struct{}` which is safe to change dynamically at runtime.
 // Unlike `pflag.StringSlice`, consecutive sets don't append to the slice, but override it.
 func DynStringSet(flagSet *flag.FlagSet, name string, value []string, usage string) *DynStringSetValue {
-	d := Dyn(flagSet, name, SetFromSlice(value), usage)
+	d := Dyn(flagSet, name, sets.FromSlice(value), usage)
 	return &DynStringSetValue{d}
 }
 
 // In order to have methods unique to this subtype... we extend/have the generic instantiated type:
 
 // DynStringSetValue implements a dynamic set of strings.
 type DynStringSetValue struct {
-	*DynValue[Set[string]]
+	*DynValue[sets.Set[string]]
 }
 
 // Contains returns whether the specified string is in the flag.
@@ -39,6 +41,6 @@ func (d *DynStringSetValue) String() string {
 	return fmt.Sprintf("%v", arr)
 }
 
-func ValidateDynStringSetMinElements(count int) func(Set[string]) error {
+func ValidateDynStringSetMinElements(count int) func(sets.Set[string]) error {
 	return ValidateDynSetMinElements[string](count)
 }

dynstringset_test.go

@@ -1,4 +1,5 @@
 // Copyright 2015 Michal Witkowski. All Rights Reserved.
+// Copyright 2020-2023 Fortio Authors. All Rights Reserved.
 // See LICENSE for licensing terms.
 
 package dflag
@@ -9,15 +10,16 @@ import (
 	"time"
 
 	"fortio.org/assert"
+	"fortio.org/sets"
 )
 
 func TestDynStringSet_SetAndGet(t *testing.T) {
 	set := flag.NewFlagSet("foobar", flag.ContinueOnError)
 	dynFlag := DynStringSet(set, "some_stringslice_1", []string{"foo", "bar"}, "Use it or lose it")
-	assert.Equal(t, Set[string]{"foo": {}, "bar": {}}, dynFlag.Get(), "value must be default after create")
+	assert.Equal(t, sets.New("foo", "bar"), dynFlag.Get(), "value must be default after create")
 	err := set.Set("some_stringslice_1", "car,bar")
 	assert.NoError(t, err, "setting value must succeed")
-	assert.Equal(t, Set[string]{"car": {}, "bar": {}}, dynFlag.Get(), "value must be set after update")
+	assert.Equal(t, sets.New("car", "bar"), dynFlag.Get(), "value must be set after update")
 }
 
 func TestDynStringSet_Contains(t *testing.T) {
@@ -45,9 +47,9 @@ func TestDynStringSet_FiresValidators(t *testing.T) {
 
 func TestDynStringSet_FiresNotifier(t *testing.T) {
 	waitCh := make(chan struct{}, 1)
-	notifier := func(oldVal Set[string], newVal Set[string]) {
-		assert.EqualValues(t, Set[string]{"foo": {}, "bar": {}}, oldVal, "old value in notify must match previous value")
-		assert.EqualValues(t, Set[string]{"car": {}, "far": {}}, newVal, "new value in notify must match set value")
+	notifier := func(oldVal sets.Set[string], newVal sets.Set[string]) {
+		assert.EqualValues(t, sets.Set[string]{"foo": {}, "bar": {}}, oldVal, "old value in notify must match previous value")
+		assert.EqualValues(t, sets.Set[string]{"car": {}, "far": {}}, newVal, "new value in notify must match set value")
 		waitCh <- struct{}{}
 	}
 

endpoint/endpoint.go

@@ -1,4 +1,5 @@
 // Copyright 2015 Michal Witkowski. All Rights Reserved.
+// Copyright 2020-2023 Fortio Authors. All Rights Reserved.
 // See LICENSE for licensing terms.
 
 package endpoint

endpoint/endpoint_test.go

@@ -97,8 +97,8 @@ func (s *endpointTestSuite) TestCorrectlyRepresentsResources() {
 		&flagJSON{
 			Name:         "some_dyn_stringslice",
 			Description:  "Some dynamic slice text",
-			CurrentValue: "[car star]",
-			DefaultValue: "[foo bar]",
+			CurrentValue: "car,star",
+			DefaultValue: "foo,bar",
 			IsChanged:    true,
 			IsDynamic:    true,
 		},

examples/server_kube/http.go

@@ -1,4 +1,5 @@
 // Copyright 2016 Michal Witkowski. All Rights Reserved.
+// Copyright 2020-2023 Fortio Authors. All Rights Reserved.
 // See LICENSE for licensing terms.
 
 package main
@@ -14,6 +15,7 @@ import (
 	"fortio.org/dflag/dynloglevel"
 	"fortio.org/dflag/endpoint"
 	"fortio.org/log"
+	"fortio.org/sets"
 )
 
 var (
@@ -47,11 +49,15 @@ var (
 			},
 		},
 		"An arbitrary JSON struct.")
+	dynArray = dflag.New([]string{"z", "b", "a"}, "An array of strings (comma separated)")
+	dynSet   = dflag.New(sets.New("z", "b", "a"), "An set of strings (comma separated)")
 )
 
 func main() {
 	dflag.FlagBool("example_bool3", dynBool3)
 	dflag.Flag("example_str2", dynStr2)
+	dflag.Flag("example_array", dynArray)
+	dflag.Flag("example_set", dynSet)
 	dynloglevel.LoggerFlagSetup()
 	flag.Parse()
 	u, err := configmap.Setup(flag.CommandLine, *dirPathWatch)

go.mod

@@ -3,10 +3,11 @@ module fortio.org/dflag
 go 1.19
 
 require (
-	fortio.org/assert v1.1.3
+	fortio.org/assert v1.1.4
 	fortio.org/log v1.2.2
+	fortio.org/sets v0.3.0
 	github.com/fsnotify/fsnotify v1.6.0
-	golang.org/x/exp v0.0.0-20230210204819-062eb4c674ab
+	golang.org/x/exp v0.0.0-20230213192124-5e25df0256eb
 )
 
 require golang.org/x/sys v0.4.0 // indirect

go.sum

@@ -1,11 +1,13 @@
-fortio.org/assert v1.1.3 h1:zXm8xiNiKvq2xG/YQ3sONAg3287XUuklKIDdjyD9pyg=
-fortio.org/assert v1.1.3/go.mod h1:039mG+/iYDPO8Ibx8TrNuJCm2T2SuhwRI3uL9nHTTls=
+fortio.org/assert v1.1.4 h1:Za1RaG+OjsTMpQS3J3UCvTF6wc4+IOHCz+jAOU37Y4o=
+fortio.org/assert v1.1.4/go.mod h1:039mG+/iYDPO8Ibx8TrNuJCm2T2SuhwRI3uL9nHTTls=
 fortio.org/log v1.2.2 h1:vs42JjNwiqbMbacittZjJE9+oi72Za6aekML9gKmILg=
 fortio.org/log v1.2.2/go.mod h1:u/8/2lyczXq52aT5Nw6reD+3cR6m/EbS2jBiIYhgiTU=
+fortio.org/sets v0.3.0 h1:StetVwMKYsatjzWuglrlElPaCA054zwiVUhDWTSg6wU=
+fortio.org/sets v0.3.0/go.mod h1:xVjulHr0FhlmReSymI+AhDtQ4FgjiazQ3JmuNpYFMs8=
 github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY=
 github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw=
-golang.org/x/exp v0.0.0-20230210204819-062eb4c674ab h1:628ME69lBm9C6JY2wXhAph/yjN3jezx1z7BIDLUwxjo=
-golang.org/x/exp v0.0.0-20230210204819-062eb4c674ab/go.mod h1:CxIveKay+FTh1D0yPZemJVgC/95VzuuOLq5Qi4xnoYc=
+golang.org/x/exp v0.0.0-20230213192124-5e25df0256eb h1:PaBZQdo+iSDyHT053FjUCgZQ/9uqVwPOcl7KSWhKn6w=
+golang.org/x/exp v0.0.0-20230213192124-5e25df0256eb/go.mod h1:CxIveKay+FTh1D0yPZemJVgC/95VzuuOLq5Qi4xnoYc=
 golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.4.0 h1:Zr2JFtRQNX3BCZ8YtxRE9hNJYC8J6I1MVbMg6owUp18=
 golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=