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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 8, 2021, 15:12 UTC
Message-ID: <xmqqsg0oc01m.fsf@gitster.g>
In-Reply-To: <93578e5a-c09b-14c2-8f87-eb354f4b576c@gmail.com>

Andrei Rybak <rybak.a.v@gmail.com> writes:

Show 12 quoted lines

> Is reusing "argv[0]" one more time instead of introducing the variable
> "page" is a good idea? It could either be:
>
> 	argv[0] = cmd_to_page(check_git_cmd(argv[0]));
>
> or
>
> 	argv[0] = check_git_cmd(argv[0]);
> 	argv[0] = cmd_to_page(argv[0]);
>
> That way, the quoted hunk above (touching calls to show_*_page) wouldn't
> be in the patch.

It is a bad idea. It gives readers a false impression that there are users of information other than show_$fmt_page() functions that do not want the original argv[0] but the variant massaged by the cmd_to_page() function later in the code after these functions return, and more importantly, it would make it impossible to recover the original value of argv[0] if the code later wants to.

To be perfectly honest, I do not see much value in the patch being discussed (I am not saying "I see no value"---just "not much") [*1*]. The patch under discussion may not be making things worse, but overwriting argv[0] WILL make it worse than the current code, I would think.

[Footnote]

*1* This is because I see nothing wrong in the original code before
    applying this patch.  How the manual pages are named after the
    actual command name may happen to be the same across all three
    show_$fmt_page() backends (and that is why they happen to all
    call cmd_to_page() helper function), but there is no strong
    reason why that has to stay that way.  E.g. "man git-cat-file"
    is used by the man backend only because cmd_to_page() yields one
    'git-cat-file' string, but "man 1 git-cat-file" would equally be
    a good way to drive the "man" viewer.  Other format backends may
    not have good use for the section information---it is more
    natural to allow each of show_$fmt_page() to use their own way
    to derive where the documentation is found for each command.