Andrei Rybak <rybak.a.v@xxxxxxxxx> writes:
> 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.
