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

From: Fernando Ramos <greenfoo@u92.eu>
Date: Dec 11, 2021, 10:46 UTC
Message-ID: <YbSBf0eLh3AXiGnV@zacax395.localdomain>
In-Reply-To: <CAJDDKr77YQKJj15V52Rs=LPRMZVSkbpV8MWFDWNuYf4jqqCajg@mail.gmail.com>

On 21/12/09 06:58PM, David Aguilar wrote:

Show 17 quoted lines

> 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.

Show 13 quoted lines

> > +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.