Usage() and PrintDefaults() write to inconsistent Writers

[ewirch]

Usage() always writes to os.Stderr:

var Usage = func() {
	fmt.Fprintf(os.Stderr, "Usage of %s:\n", os.Args[0])
	PrintDefaults()
}

PrintDefaults() writes to Commandline.out():

func PrintDefaults() {
	CommandLine.PrintDefaults()
}
func (f *FlagSet) PrintDefaults() {
	usages := f.FlagUsages()
	fmt.Fprint(f.out(), usages)
}

[mckern]

This is, to some degree, mitigated by #220 (and #241) since they'd allow you to define the output before emitting the usage header the same way that pkg/flag does. I think that #230 is somewhat related to this too.

[alessio]

Usage() always writes to os.Stderr:

Whilst Parse() writes to stdout unconditionally on f.errorHandling = ExitOnError. I think #263 somehow relates to this too.

[mohkale]

IMO the error should be passed to the Usage function. Or at the vary least a parameter should be added to indicate that help is being printed or an actual error happened.

A common use case with cli programs is to print help and then pipe it to a PAGER.

./foo -h | less

Which means when the user asks for it, it should be printed to stdout. Otherwise users will have to add a needless redirect.

./foo -h 2>&1 | less

But when there's an actual parse error, it should print to stderr or else the help message could affect results in later pipelines.

./foo --this-isnt-a-valid-flag | while read line; do
  # do something with the output of foo
done

[cornfeedhobo]

@mohkale A little swamped, but I'm going to take a look at this suggestion soon

[mohkale]

@cornfeedhobo might I suggest passing the stream that help should be written to to the help function. That way there's very little chance some user who doesn't have a good grasp on how the stdin,out,err concept works will accidentally end up using the wrong stream.

Something like:

var Usage = func(fd *io.Writer) {
	fmt.Fprintf(fd, "Usage of %s:\n", os.Args[0])
	PrintDefaults(fd)
}

We don't need to know what happened, just write the usage like you normally would.

[cornfeedhobo]

It appears that the core flag library has already fixed this and uses CommandLine.Output(). As usual, I'll apply this to my fork and anyone is welcome to use that instead.

[mohkale]

Sounds good to me, why not open a pull request?

[cornfeedhobo]

@mohkale I have 3 that are open and they are two years old now. Maybe I'll try merging my work back in, but I'm already a few commits ahead at this point. Giving them the benefit of the doubt that they are just busy, it's best I fork and see how it goes. I've already merged in a bunch of fixes and am looking into adding more regression tests soon.

[mohkale]

I see, totally understand. Thnx for the great work. 😄.