Re: [PATCH resend] help: convert git_cmd to page in one place

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

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> @@ -606,16 +604,17 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>>  
>>  	argv[0] = check_git_cmd(argv[0]);
>>  
>> +	page = cmd_to_page(argv[0]);
>
> Nit not requring a re-roll: I'd snuggle this with the argv[0], not the
> switch statement, i.e. like the existing code.

Makes sense.

>>  	switch (help_format) {
>>  	case HELP_FORMAT_NONE:
>>  	case HELP_FORMAT_MAN:
>> -		show_man_page(argv[0]);
>> +		show_man_page(page);
>>  		break;
>>  	case HELP_FORMAT_INFO:
>> -		show_info_page(argv[0]);
>> +		show_info_page(page);
>>  		break;
>>  	case HELP_FORMAT_WEB:
>> -		show_html_page(argv[0]);
>> +		show_html_page(page);
>>  		break;
>>  	}
>
> More generally (not the fault of this patch) the control flow in that
> file is quite a mess. I wondered why we can't just add this to
> check_git_cmd() then, it's also only called in that one place.
>
> We can, but it and cmd_to_page() return in multiple places, and
> help_unknown_cmd() might return, might exit(1) itself etc, so fixing
> similar to my 338abb0f045 (builtins + test helpers: use return instead
> of exit() in cmd_*, 2021-06-08) should probably be part of some general
> refactoring... :)

True, too.