Re: [PATCH v3 4/6] usage: add show_usage_and_exit_if_asked()

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

>> 	show_usage_and_exit_if_asked(argc, argv, usage);
>> 
>> to help correct these code paths.
>
> I found the name hard to distinguish from the earlier helper,

As we all know that usage_with_options() and usage() exits,

    show_usage_with_options_if_asked()
    show_usage_if_asked()

may be good enough, I wonder?  "show" -> "help" may give us better
names.

> I think the "and_exit" could probably be implied, since showing the
> usage is always a final thing (just like in usage() and
> usage_with_options()). So:
>
>   show_usage_if_asked();
>   show_usage_with_options_if_asked();

Ah, We came to the same conclusion.  The only thing we haven't made
it obvious is that "show" implies the output goes to the standard
output stream, while without "show", the output goes to the standard
error stream.  I wonder if these help better:

    show_help_if_asked();
    show_help_with_options_if_asked()

"show help" explains the output goes to where the "help" message
would go ;-)

But probably "if-asked" is clear enough indication that the output
is made in response to an explicit end-user request, so let's take
the show_(usage|usage_with_options)_if_asked as the final names.

>> -static void vreportf(const char *prefix, const char *err, va_list params)
>> +static void vfdreportf(int fd, const char *prefix, const char *err, va_list params)
>>  {
>>  	char msg[4096];
>>  	char *p, *pend = msg + sizeof(msg);
>> @@ -32,8 +32,14 @@ static void vreportf(const char *prefix, const char *err, va_list params)
>>  	}
>>  
>>  	*(p++) = '\n'; /* we no longer need a NUL */
>> -	fflush(stderr);
>> -	write_in_full(2, msg, p - msg);
>> +	if (fd == 2)
>> +		fflush(stderr);
>> +	write_in_full(fd, msg, p - msg);
>> +}
>
> Gross. :) I think the existing code is conceptually:
>
>   write_in_full(fileno(stderr), msg, p - msg);
>
> In which case vfreportf() could just take a FILE*, flush it and then
> write.

Sure, but these "stderr" are real error reporting that need to stay
to be stderr, and flush needs to be done only when our true payload
goes to fd#2 and I do not think these fflush() are about stdio calls
made by the caller _before_ it called this function.  It may become
a bit tricky to read the resulting code if we pass "FILE *".