git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
	"Martin Ågren" <martin.agren@gmail.com>
Subject: Re: [PATCH] doc: remove custom callouts format
Date: Tue, 18 Apr 2023 00:00:34 -0400	[thread overview]
Message-ID: <20230418040034.GC60552@coredump.intra.peff.net> (raw)
In-Reply-To: <20230418011828.47851-1-felipe.contreras@gmail.com>

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

> 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

  reply	other threads:[~2023-04-18  4:00 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-18  1:18 [PATCH] doc: remove custom callouts format Felipe Contreras
2023-04-18  4:00 ` Jeff King [this message]
2023-04-18  4:41   ` Jeff King
2023-04-18  7:25     ` Felipe Contreras
2023-04-18  5:30   ` Felipe Contreras
2023-04-18  6:17     ` Jeff King
2023-04-18  7:15       ` Felipe Contreras
2023-04-18  9:03         ` Jeff King
2023-04-18  9:50           ` Felipe Contreras

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230418040034.GC60552@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=martin.agren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).