Fix stderr printing functions

[alessio]

Follow-up of #822

This fixes stderr redirection.

Closes: #1100

@spf13 @eparis

[alessio]

CC'ing @jleni

[amdprophet]

Should https://github.com/spf13/cobra/blob/master/command.go#L926 use PrintErr? We're seeing errors output to stdout after calling SetOut and SetErr.

[alessio]

Yes, I think it should. Will amend my PR ASAP

[alessio]

@amdprophet done, please review

[jleni]

@alessio for some reason, the changes seem to break TestBashCompletions

[alessio]

Will try to reproduce and fix shortly

[PapaCharlie]

The shellcheck warnings are being addressed in #889

[alessio]

Point taken: mapfile is available in Bash >= 4. Happy to incorporate #889 commits instead of going for my solution

[alessio]

Conflicts have been resolved, please review

[portertech]

Please review 🙏

[amdprophet]

@spf13 @eparis @jleni any chance of this getting reviewed soon? 🙏

[alessio]

Hi!

Any news? @spf13 @eparis @jleni

[jleni]

Sorry @alessio.. but there isn't much I can do myself! I don't have any permissions in this repo

[alessio]

@jharshman bump :)

[portertech]

@jharshman bump :)

[alessio]

@jharshman Hi! Any news on this? :)

[alessio]

@marckhouzam bump :)

[marckhouzam]

@marckhouzam bump :)

I'm nobody 😉

But if I may suggest something based on my experience. To help get any PR merged (in any project) you should make the review as easy as possible for the reviewer. So, in the PR description, you really need to explain what problem this fixes. If you can mention why this is valuable (the impact of the bug), it would also help prioritize this PR over the many others. Finally, mentioning if there are any backwards-compatibility issues or not would probably help build confidence in your change (that's what makes me unsure of this change, considering my limited experience with Cobra).

Just my opinion though.

[alessio]

I amended the PR as requested. I've just tweaked the README.me file (error messages should always go to STDERR, even when a program fails).

@jharshman @marckhouzam please review again when you spare some time.

Thanks for considering.

[fedekunze]

Thanks for the fix @alessio. I have run into the same issue

[ethanfrey]

Thank you, this would really help us with proper output so we can use the cli tools in scripts more easily.

[jgimeno]

I have been waiting this solution for a long time, how come is still not merged? I hope this is being merged soon.

I don't know what is the problem to release this in a minor version (patch will be more accurate). Only people that upgrades will see the problem and other will update when they have bandwidth to solve the issue.

[jpmcb]

This looks good to me! Thanks for waiting on this. We will get this merged very soon! 🚀

[bukowa]

func TestCommand_Print(t *testing.T) {
	errBuff, outBuff := bytes.NewBuffer(nil), bytes.NewBuffer(nil)

	wantErr := []byte("PrintErrPrintErr line\nPrintErr")
	wantOut := []byte("PrintPrint line\nPrint")

	root := &Command{
		Run: func(cmd *Command, args []string) {

			cmd.PrintErr("PrintErr")
			cmd.PrintErrln("PrintErr", "line")
			cmd.PrintErrf("PrintEr%s", "r")

			cmd.Print("Print")
			cmd.Println("Print", "line")
			cmd.Printf("Prin%s", "t")
		},
	}

	root.SetErr(errBuff)
	root.SetOut(outBuff)

	if err := root.Execute(); err != nil {
		t.Error(err)
	}

	errBytes, err := ioutil.ReadAll(errBuff)
	if err != nil {
		t.Error(err)
	}

	outBytes, err := ioutil.ReadAll(outBuff)
	if err != nil {
		t.Error(err)
	}

	if !bytes.Equal(errBytes, wantErr) {
		t.Errorf("got: '%s' want: '%s'", errBytes, wantErr)
	}

	if !bytes.Equal(outBytes, wantOut) {
		t.Errorf("got: '%s' want: '%s'", outBytes, wantOut)
	}
}

[alessio]

@bukowa I've integrated your test case in this PR. Thanks!

[jpmcb]

This is great - thanks so much for the patience and time spent on this. And thanks so much for adding a test case!