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

flag: PrintDefaults does not print default of "0" for String flags #23543

Closed
spenczar opened this issue Jan 24, 2018 · 4 comments
Closed

flag: PrintDefaults does not print default of "0" for String flags #23543

spenczar opened this issue Jan 24, 2018 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@spenczar
Copy link
Contributor

What version of Go are you using (go version)?

go version go1.9.3 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

darwin/amd64

What did you do?

https://play.golang.org/p/F1sa0z0p9V1:

package main

import (
	"flag"
)

var (
	aaa = flag.String("aaa", "", "Set aaa")
	bbb = flag.String("bbb", "0", "Set bbb")
	ccc = flag.String("ccc", "1", "Set ccc")
)

func main() {
	flag.PrintDefaults()
}

What did you expect to see?

  -aaa string
    	Set aaa
  -bbb string
    	Set bbb (default "0")
  -ccc string
    	Set ccc (default "1")

What did you see instead?

  -aaa string
    	Set aaa
  -bbb string
    	Set bbb
  -ccc string
    	Set ccc (default "1")

This is fairly explicit in the relevant code, flag.isZeroValue. The documentation says that "The parenthetical default is omitted if the default is the zero value for the type," but "0" is not the zero value of string.

@garethlewin
Copy link

I believe that the switch statement in isZeroValue is not needed, I can't create a reproduction case that fails the top part and needs the switch case.

@ianlancetaylor
Copy link
Contributor

The handling of "0" dates back to https://golang.org/cl/7330 when the isZeroValue function was introduced. It was preserved by https://golang.org/cl/23581 which added the reflect based code.

I would be OK with removing the switch. It's not clear that it's needed any more. CC @robpike

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 24, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Jan 24, 2018
@robpike
Copy link
Contributor

robpike commented Jan 24, 2018

As Ian said, but I think in error, it's NOT clear that it's not needed any more. I do think it's safe to remove, though.

We can try it in 1.11 and see what breaks.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/103867 mentions this issue: flag: correct zero values when printing defaults

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants