Re: [PATCH] doc: remove custom callouts format

From: Jeff King <peff@peff.net>
Date: Apr 18, 2023, 04:00 UTC
Message-ID: <20230418040034.GC60552@coredump.intra.peff.net>
In-Reply-To: <20230418011828.47851-1-felipe.contreras@gmail.com>

On Mon, Apr 17, 2023 at 07:18:28PM -0600, Felipe Contreras wrote:

Show 28 quoted lines

> What's worse: the format of the upstream callouts is much nicer than our
> hacked version.
> 
> Compare this:
> 
>      $ git diff            (1)
>      $ git diff --cached   (2)
>      $ git diff HEAD       (3)
> 
>   1. Changes in the working tree not yet staged for the next
>      commit.
>   2. Changes between the index and your last commit; what you
>      would be committing if you run git commit without -a
>      option.
>   3. Changes in the working tree since your last commit; what
>      you would be committing if you run git commit -a
> 
> To this:
> 
>      $ git diff            (1)
>      $ git diff --cached   (2)
>      $ git diff HEAD       (3)
> 
>  1. Changes in the working tree not yet staged for the next commit.
>  2. Changes between the index and your last commit; what you would
>  be committing if you run git commit without -a option.
>  3. Changes in the working tree since your last commit; what you
>  would be committing if you run git commit -a

You don't say which is which, so I'm hoping that the top one is the new output. :) And running doc-diff shows that it is. Good.

It does look like at least one spot is made worse, though. In git-checkout, we have now:

          1. The following sequence checks out the master branch,
             reverts the Makefile to two revisions back, deletes hello.c
             by mistake, and gets it back from the index.
  
                 $ git checkout master             (1)
                 $ git checkout master~2 Makefile  (2)
                 $ rm -f hello.c
                 $ git checkout hello.c            (3)
  
             1. switch branch
             2. take a file out of another commit
             3. restore hello.c from the index
  
             If you want to check out all C source files out of the
             index, you can say
  
                 $ git checkout -- '*.c'
  
             Note the quotes around *.c. The file hello.c will also be
             checked out, even though it is no longer in the working
             tree, because the file globbing is used to match entries in
             the index (not in the working tree by the shell).

which is achieved through plus-continuation of each paragraph, to make it all part of the block under item "1." in the numbered list.

But after your patch, it's:

          1. The following sequence checks out the master branch,
             reverts the Makefile to two revisions back, deletes hello.c
             by mistake, and gets it back from the index.
  
                 $ git checkout master             (1)
                 $ git checkout master~2 Makefile  (2)
                 $ rm -f hello.c
                 $ git checkout hello.c            (3)
  
              1. switch branch
              2. take a file out of another commit
              3. restore hello.c from the index
  
                 If you want to check out all C source files out of
                 the index, you can say
  
                                $ git checkout -- '*.c'
  
                            Note the quotes around *.c. The file
                            hello.c will also be checked out, even
                            though it is no longer in the working
                            tree, because the file globbing is used
                            to match entries in the index (not in the
                            working tree by the shell).

The plus-continuation seems to get attached to the final item of the callout list, rather than to the surrounding block. I tried one or two things to disambiguate this (like opening a new block around the example+callout section), but couldn't get any reasonable output. It's also weird that it keeps indenting each block further (I'd expect "Note the quotes" to be in line with "If you want to").

This is perhaps a bug in asciidoc itself. Building with USE_ASCIIDOCTOR is mostly good, though it seems to drop the newline between the callout list and the next paragraph, which our custom one has:

          1. The following sequence checks out the master branch,
             reverts the Makefile to two revisions back, deletes hello.c
             by mistake, and gets it back from the index.
  
                 $ git checkout master             (1)
                 $ git checkout master~2 Makefile  (2)
                 $ rm -f hello.c
                 $ git checkout hello.c            (3)
  
              1. switch branch
              2. take a file out of another commit
              3. restore hello.c from the index
             If you want to check out all C source files out of the
             index, you can say
  
                 $ git checkout -- '*.c'
  
             Note the quotes around *.c. The file hello.c will also be
             checked out, even though it is no longer in the working
             tree, because the file globbing is used to match entries in
             the index (not in the working tree by the shell).

It's probably still worth moving forward with your patch, as I think it takes us in the direction we want long-term (and which builds with asciidoctor are already using). But we may want to pair it with a patch to work around the issue with git-checkout.1 using asciidoc to avoid regressing that section. It may require re-wording or re-organizing to work around the bug.

-Peff