Re: [PATCH v4 2/3] vimdiff: add tool documentation

> On 21/12/09 06:58PM, David Aguilar wrote:
> > On Sat, Dec 4, 2021 at 1:04 AM Fernando Ramos <greenfoo@u92.eu> wrote:
> > >
> > > Running 'git {merge,diff}tool --tool-help' now also prints usage
> > > information about the vimdiff tool (and its variantes) instead of just
> > > its name.
> > >
> > > Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> > > ---
> > >  Documentation/git-difftool--vimdiff.txt  |  40 +++++
> > >  Documentation/git-mergetool--vimdiff.txt | 195 +++++++++++++++++++++++
> > 
> > 
> > Should these be referenced from elsewhere in the Documentation/
> > tree in order to make them more discoverable?
> > 
> > Documentation/git-{difftool,mergetool}.txt seem like they should
> > include or link to these.
> 
> Those files (Documentation/git-{difftool,mergetool}.txt) contain this piece of
> text:
> 
>     --tool-help
>         Print a list of diff tools that may be used with --tool.
> 
> When you then run with "--tool-help", not only the list of available tools is
> listed, but also a short message explaining how to obtain more details is
> presented (for those tools that have more information):
> 
>     vimdiff 
>         Run 'man git-difftool--vimdiff' for details
> 
> In other words: there is one level of "indirection" that decouples the
> documentation of each tool from the generic documentation contained in
> Documentation/git-{difftool,mergetool}.txt.
> 
> Thanks to this, when documentation for a new tool is created in the future, as
> long as the "diff_cmd_help()/merge_cmd_help()" functions are overwritten,
> everything will "just work" without having to edit
> Documentation/git-{difftool,mergetool}.txt each time.
> 
> I thought this was the best option but I have not problem adding a reference to
> it if you prefer that (maybe at the end, in the "SEE ALSO" section?)
> 
> Let me know what you think.
> 
> 
> > > +merge_cmd_help() {
> > > +       echo "Run 'man git-mergetool--vimdiff' for details"
> > > +       return 0
> > > +}
> > > +
> > > +
> > 
> > My understanding is that we prefer "git help" instead of "man" when
> > providing hints.
> > 
> > That means we should suggest something like this instead:
> > 
> >         echo "Run 'git help mergetool--vimdiff' for details"
> 
> I originally typed that, but then I noticed something in my system: all entries
> in "Documentation/" seem to be available as "man" pages, but only a subset of
> them are also available through "git help". Examples:
> 
>   * man git-mergetool        --> works
>   * man git-mergetool--lib   --> works
> 
>   * git help mergetool       --> works
>   * git help mergetool--lib  --> does not work!
> 
> That's why I ended up using "man".
> 
> PS: Now that I have revisited the issue, it looks like the command that is
> failing is trying to run "man gitmergetool--lib" under the hood (notice there is
> a missing "-"). The problem seems to be in "builtin/help.c:cmd_to_page()":
> 
>    if is_git_command(x) 
>        return xstrfmt("git-%s", x);
>    else
>        return xstrfmt("git%s", x);
> 
> Because "mergetool--lib" is not a regular git command, it removes the "-". The
> same would happen to the new "git help mergetool--vimdiff" if I add it. Is
> this the intended behavior?
> 
> There are two options:
> 
>   A) This is actually an error in help.c logic that needs to be fixed and then
>      I can update the docs to "git help mergetool--vimdiff".
> 
>   B) This is the intended behavior and I leave "man git-mergetool--vimdiff" in
>      the docs.
> 
> I would say the right option is (A), but need your input regarding how to fix
> it.
> 
> Thanks.
>