Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide Default Value in usage for Value's with irregular zero values #204

Open
AdamSLevy opened this issue Apr 24, 2019 · 1 comment · May be fixed by #205
Open

Hide Default Value in usage for Value's with irregular zero values #204

AdamSLevy opened this issue Apr 24, 2019 · 1 comment · May be fixed by #205

Comments

@AdamSLevy
Copy link

AdamSLevy commented Apr 24, 2019

Currently default values that are zero are not displayed in usage.

Value interfaces whose String() string method doesn't output "0", "", "", or "false" for their uninitialized value aren't considered zero valued.

pflag/flag.go

Lines 519 to 547 in 298182f

func (f *Flag) defaultIsZeroValue() bool {
switch f.Value.(type) {
case boolFlag:
return f.DefValue == "false"
case *durationValue:
// Beginning in Go 1.7, duration zero values are "0s"
return f.DefValue == "0" || f.DefValue == "0s"
case *intValue, *int8Value, *int32Value, *int64Value, *uintValue, *uint8Value, *uint16Value, *uint32Value, *uint64Value, *countValue, *float32Value, *float64Value:
return f.DefValue == "0"
case *stringValue:
return f.DefValue == ""
case *ipValue, *ipMaskValue, *ipNetValue:
return f.DefValue == "<nil>"
case *intSliceValue, *stringSliceValue, *stringArrayValue:
return f.DefValue == "[]"
default:
switch f.Value.String() {
case "false":
return true
case "<nil>":
return true
case "":
return true
case "0":
return true
}
return false
}
}

There are two simple ways that I see to solve this. One is to use Flag.DefValue instead of Flag.Value.String() in the nested switch statement in the default case. This appears to be the most consistent way to program this as all of the other cases are comparing Flag.DefValue with their type's zero value. Of course Flag.DefValue is set from Flag.Value.String() when the flag is first initialized, so this should have no effect on existing code.

pflag/flag.go

Line 817 in 298182f

DefValue: value.String(),

I personally feel that the use of Flag.Value.String() in the nested switch statement is incorrect code, but it's a bit subjective.

Using Flag.DefValue instead is a small change, but it will allow a user to override Flag.DefValue to the empty string if they wish to hide the "(default %v)" in the usage description for the flag.

The other way, more obtrusive but also more flexible way, to achieve this is by adding an explicit option to the Flag struct that will hide the default value and skip the zero value check altogether.

pflag/flag.go

Line 712 in 298182f

if !flag.defaultIsZeroValue() {

Something like

type Flag struct {
...
	HideDefaultValue bool
}
...
func (f *FlagSet) FlagUsagesWrapped(cols int) string {
...
		if !flag.HideDefaultValue && !flag.defaultIsZeroValue() {
			if flag.Value.Type() == "string" {
				line += fmt.Sprintf(" (default %q)", flag.DefValue)
			} else {
				line += fmt.Sprintf(" (default %s)", flag.DefValue)
			}
		}
AdamSLevy added a commit to AdamSLevy/pflag that referenced this issue Apr 24, 2019
AdamSLevy added a commit to AdamSLevy/pflag that referenced this issue Apr 24, 2019
@AdamSLevy AdamSLevy linked a pull request Apr 24, 2019 that will close this issue
AdamSLevy added a commit to AdamSLevy/pflag that referenced this issue Dec 4, 2019
@basilgello
Copy link

@spf13 it would be also great to add a configuration option to disable appending default values:

https://github.com/spf13/pflag/blob/master/flag.go#L724

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants