Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions conf.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/url"
"os"
"reflect"
"sort"
"strings"
)

Expand Down Expand Up @@ -75,13 +76,16 @@ func String(v interface{}) (string, error) {
return "", err
}

sf := sortedFields{fields: fields}
sort.Sort(&sf)

var s strings.Builder
for i, fld := range fields {
for i, fld := range sf.fields {
if fld.Options.Noprint {
continue
}

s.WriteString(flagUsage(fld))
s.WriteString(longOptInfo(fld))
s.WriteString("=")
v := fmt.Sprintf("%v", fld.Field.Interface())

Expand Down
129 changes: 88 additions & 41 deletions conf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,7 @@ func TestUsage(t *testing.T) {
t.Log(diff)
t.Log("GOT:\n", got)
t.Log("EXP:\n", tt.want)
return
}
t.Logf("\t%s\tShould match byte for byte the output.", success)
}
Expand All @@ -605,6 +606,7 @@ func TestUsage(t *testing.T) {
t.Log(diff)
t.Log("GOT:\n", got)
t.Log("EXP:\n", tt.options)
return
}
t.Logf("\t%s\tShould match byte for byte the output.", success)
}
Expand All @@ -615,55 +617,98 @@ func TestUsage(t *testing.T) {
}
}

var expectedStringOutput = ` --a-string=B
--an-int=1
--bool=true
--custom=@hello@
--debug-host=http://xxxxxx:xxxxxx@0.0.0.0:4000
--e-dur=1m0s
--immutable=mydefaultvalue
--ip-endpoints=[127.0.0.1:200 127.0.0.1:829]
--ip-ip=127.0.0.0
--ip-name=localhost
--name=andy
--password=xxxxxx`

func TestExampleString(t *testing.T) {
tt := struct {
envs map[string]string
tests := []struct {
name string
namespace string
want string
envs map[string]string
}{
envs: map[string]string{
"TEST_AN_INT": "1",
"TEST_S": "s",
"TEST_BOOL": "TRUE",
"TEST_SKIP": "SKIP",
"TEST_IP_NAME": "local",
"TEST_NAME": "andy",
"TEST_DURATION": "1m",
{
name: "with-namespace",
namespace: "TEST",
envs: map[string]string{
"TEST_AN_INT": "1",
"TEST_S": "s",
"TEST_BOOL": "TRUE",
"TEST_SKIP": "SKIP",
"TEST_IP_NAME": "local",
"TEST_NAME": "andy",
"TEST_DURATION": "1m",
},
want: expectedStringOutput,
},
{
name: "without-namespace",
namespace: "",
envs: map[string]string{
"AN_INT": "1",
"S": "s",
"BOOL": "TRUE",
"SKIP": "SKIP",
"IP_NAME": "local",
"NAME": "andy",
"DURATION": "1m",
},
want: expectedStringOutput,
},
}

os.Clearenv()
for k, v := range tt.envs {
os.Setenv(k, v)
}
t.Log("Given the need validate conf output.")
{
for testID, tt := range tests {
f := func(t *testing.T) {
t.Logf("\tTest: %d\tWhen testing %s", testID, tt.name)
{
os.Clearenv()
for k, v := range tt.envs {
os.Setenv(k, v)
}

os.Args = []string{"conf.test"}
os.Args = []string{"conf.test"}

var cfg config
if _, err := conf.Parse("TEST", &cfg); err != nil {
fmt.Print(err)
return
}
var cfg config
if _, err := conf.Parse(tt.namespace, &cfg); err != nil {
t.Log(err)
return
}

out, err := conf.String(&cfg)
if err != nil {
fmt.Print(err)
return
}
got, err := conf.String(&cfg)
if err != nil {
t.Log(err)
return
}

got = strings.TrimRight(got, "\n")
gotS := strings.Split(got, "\n")
wantS := strings.Split(tt.want, "\n")
if diff := cmp.Diff(gotS, wantS); diff != "" {
t.Errorf("\t%s\tShould match the output byte for byte. See diff:", failed)
t.Log(diff)
t.Log("GOT:\n", got)
t.Log("EXP:\n", tt.want)
return
}
t.Logf("\t%s\tShould match byte for byte the output.", success)
}
}

fmt.Print(out)

// Output:
// --an-int=1
// --a-string/-s=B
// --bool=true
// --ip-name=localhost
// --ip-ip=127.0.0.0
// --ip-endpoints=[127.0.0.1:200 127.0.0.1:829]
// --debug-host=http://xxxxxx:xxxxxx@0.0.0.0:4000
// --password=xxxxxx
// --immutable=mydefaultvalue
// --custom=@hello@
// --name=andy
// --e-dur/-d=1m0s
t.Run(tt.name, f)
}
}
}

type ConfExplicit struct {
Expand Down Expand Up @@ -758,10 +803,11 @@ func TestVersionExplicit(t *testing.T) {
f := func(t *testing.T) {
os.Args = tt.args
if help, err := conf.Parse("APP", &tt.config); err != nil {
if err == conf.ErrHelpWanted {
if err != conf.ErrHelpWanted {
if diff := cmp.Diff(tt.want, help); diff != "" {
t.Errorf("\t%s\tShould match the output byte for byte. See diff:", failed)
t.Log(diff)
return
}
t.Logf("\t%s\tShould match byte for byte the output.", success)
}
Expand Down Expand Up @@ -860,6 +906,7 @@ func TestVersionImplicit(t *testing.T) {
if diff := cmp.Diff(tt.want, help); diff != "" {
t.Errorf("\t%s\tShould match the output byte for byte. See diff:", failed)
t.Log(diff)
return
}
t.Logf("\t%s\tShould match byte for byte the output.", success)
}
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,5 @@ go 1.23.0

require (
github.com/google/go-cmp v0.3.1
golang.org/x/text v0.23.0
gopkg.in/yaml.v3 v3.0.1
)
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg=
github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
golang.org/x/text v0.23.0 h1:D71I7dUrlY+VX0gQShAThNGHFxZ13dGLBHQLVl1mJlY=
golang.org/x/text v0.23.0/go.mod h1:/BLNzu4aZCJ1+kcD0DNRotWKage4q2rGVAg4o22unh4=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
Expand Down
5 changes: 5 additions & 0 deletions sources.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,11 @@ func flagUsage(fld Field) string {
return usage
}

// longOptInfo constructs a long option description string.
func longOptInfo(fld Field) string {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why add a new function when flagUsage won't be used anymore? Why not change change the flagUsage function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flagUsage() used in writeOptions() and it's functionality contains short flags for help output. So I decided to create a new function.

But I think both flagUsage() and longOptInfo() have two responsibilities. Would be better if they provided flag info only without formatting with spaces. But may be this is idea for next PR :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hate these PR's because it's so hard to see everything. I will have time on Friday to properly review and publish this. Thanks

return " --" + strings.ToLower(strings.Join(fld.FlagKey, `-`))
}

/*
Portions Copyright (c) 2009 The Go Authors. All rights reserved.

Expand Down
14 changes: 7 additions & 7 deletions usage.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@ import (
"os"
"path"
"reflect"
"sort"
"strings"
"text/tabwriter"

"golang.org/x/text/collate"
"golang.org/x/text/language"
)

const (
Expand All @@ -31,8 +29,11 @@ func (sf sortedFields) Swap(i, j int) {
sf.fields[i], sf.fields[j] = sf.fields[j], sf.fields[i]
}

func (sf sortedFields) Bytes(i int) []byte {
return []byte(strings.ToLower(strings.Join(sf.fields[i].FlagKey, `-`)))
func (sf sortedFields) Less(i, j int) bool {
s1 := strings.ToLower(strings.Join(sf.fields[i].FlagKey, `-`))
s2 := strings.ToLower(strings.Join(sf.fields[j].FlagKey, `-`))

return s1 < s2
}

func containsField(fields []Field, name string) bool {
Expand Down Expand Up @@ -70,8 +71,7 @@ func fmtUsage(namespace string, fields []Field) string {
}

sf := sortedFields{fields: fields}
cc := collate.New(language.English)
cc.Sort(sf)
sort.Sort(&sf)

_, file := path.Split(os.Args[0])
fmt.Fprintf(&sb, "Usage: %s [options...] [arguments...]\n\n", file)
Expand Down
Loading