don't print to stdout unconditionally on ExitOnError

[alessio]

If flagset.output was set to stderr, the error would still be printed to stdout instead of stderr.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

Alessio Treglia seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[mckern]

Can you clarify intent a little bit? "If flagset.output was set to stderr, the error would still be printed to stderr instead of stdout" still seems like desired and explicit behavior to me; what are the circumstances where a fatal error should print to stdout instead of stderr?

[alessio]

"If flagset.output was set to stderr, the error would still be printed to stderr instead of stdout"

It was a dumb typo, thanks for pointing it out. I've just fixed the PR's description. Obviously the problem is that on ExitOnError f.Parse() would print the error message to STDOUT instead of f.Output().

[alessio]

CC'ing @therealmitchconnors

[cornfeedhobo]

@alessio I'm going to take a look, but if you can think of a test for this, that'd be great!

[alessio]

@cornfeedhobo how about the following:

func TestParseErrorHandling(t *testing.T) {
	if os.Getenv("PFLAG_CRASH_TEST") == "1" {
		f := NewFlagSet(t.Name(), ExitOnError)
		args := []string{"--invalid-arg"}

		f.Parse(args)
		t.Fatal("this error should not be triggered")

		return
	}

	cmd := exec.Command(os.Args[0], "-test.run="+t.Name())
	mockStderr := bytes.NewBufferString("")
	mockStdout := bytes.NewBufferString("")
	cmd.Env = append(os.Environ(), "PFLAG_CRASH_TEST=1")
	cmd.Stderr = mockStderr
	cmd.Stdout = mockStdout
	err := cmd.Run()

	if e, ok := err.(*exec.ExitError); ok && !e.Success() {
		want := `unknown flag: --invalid-arg
Usage of TestParseErrorHandling:
unknown flag: --invalid-arg
`
		if got := mockStderr.String(); got != want {
			t.Errorf("got '%s', want '%s'", got, want)
		}

		if len(mockStdout.String()) != 0 {
			t.Error("stdout should be empty")
		}

		return
	}

	t.Fatal("this error should not be triggered")
}

[dmitryrn]

Printing to STDOUT without new line prevents Goland to print test output correctly https://youtrack.jetbrains.com/issue/GO-7215 :(