{"thread":{"id":"59613","subject":"[PATCH] doc: remove custom callouts format","startedAt":"2023-04-18T01:18:52Z","lastAt":"2023-04-18T09:50:21Z","messageCount":9,"participants":["Felipe Contreras","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"475571","messageId":"20230418011828.47851-1-felipe.contreras@gmail.com","threadId":"59613","inReplyTo":null,"subject":"[PATCH] doc: remove custom callouts format","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-04-18T01:18:28Z","receivedAt":"2023-04-18T01:18:52Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The code to render callouts for manpages comes from 17 years ago:\n776e994af5 (Properly render asciidoc \"callouts\" in git man pages.,\n2006-04-28), and it was needed back then, but DocBook Stylesheets added\nsupport for that in 2008 [1], since 1.74.0 it hasn't been necessary.\n\nWhat's worse: the format of the upstream callouts is much nicer than our\nhacked version.\n\nCompare this:\n\n $ git diff (1)\n $ git diff --cached (2)\n $ git diff HEAD (3)\n\n 1. Changes in the working tree not yet staged for the next\n commit.\n 2. Changes between the index and your last commit; what you\n would be committing if you run git commit without -a\n option.\n 3. Changes in the working tree since your last commit; what\n you would be committing if you run git commit -a\n\nTo this:\n\n $ git diff (1)\n $ git diff --cached (2)\n $ git diff HEAD (3)\n\n 1. Changes in the working tree not yet staged for the next commit.\n 2. Changes between the index and your last commit; what you would\n be committing if you run git commit without -a option.\n 3. Changes in the working tree since your last commit; what you\n would be committing if you run git commit -a\n\nLet's drop our unnecessary inferior custom format and use the official\none.\n\n[1] https://sourceforge.net/p/docbook/code/7842/\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/manpage-normal.xsl | 15 ---------------\n 1 file changed, 15 deletions(-)\n\ndiff --git a/Documentation/manpage-normal.xsl b/Documentation/manpage-normal.xsl\nindex a9c7ec69f4..e4c5874ed3 100644\n--- a/Documentation/manpage-normal.xsl\n+++ b/Documentation/manpage-normal.xsl\n@@ -8,19 +8,4 @@\n \n \n \n-\n-\n-\t\n-\n-\n-\t.sp \n-\t\n-\t \n-\n-\n-\t\n-\t\n-\t.br \n-\n-\n \n-- \n2.40.0+fc1\n\n"},{"id":"475582","messageId":"20230418040034.GC60552@coredump.intra.peff.net","threadId":"59613","inReplyTo":"20230418011828.47851-1-felipe.contreras@gmail.com","subject":"Re: [PATCH] doc: remove custom callouts format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-04-18T04:00:34Z","receivedAt":"2023-04-18T04:00:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 17, 2023 at 07:18:28PM -0600, Felipe Contreras wrote:\n\n> What's worse: the format of the upstream callouts is much nicer than our\n> hacked version.\n> \n> Compare this:\n> \n> $ git diff (1)\n> $ git diff --cached (2)\n> $ git diff HEAD (3)\n> \n> 1. Changes in the working tree not yet staged for the next\n> commit.\n> 2. Changes between the index and your last commit; what you\n> would be committing if you run git commit without -a\n> option.\n> 3. Changes in the working tree since your last commit; what\n> you would be committing if you run git commit -a\n> \n> To this:\n> \n> $ git diff (1)\n> $ git diff --cached (2)\n> $ git diff HEAD (3)\n> \n> 1. Changes in the working tree not yet staged for the next commit.\n> 2. Changes between the index and your last commit; what you would\n> be committing if you run git commit without -a option.\n> 3. Changes in the working tree since your last commit; what you\n> would be committing if you run git commit -a\n\nYou don't say which is which, so I'm hoping that the top one is the new\noutput. :) And running doc-diff shows that it is. Good.\n\nIt does look like at least one spot is made worse, though. In\ngit-checkout, we have now:\n\n 1. The following sequence checks out the master branch,\n reverts the Makefile to two revisions back, deletes hello.c\n by mistake, and gets it back from the index.\n \n $ git checkout master (1)\n $ git checkout master~2 Makefile (2)\n $ rm -f hello.c\n $ git checkout hello.c (3)\n \n 1. switch branch\n 2. take a file out of another commit\n 3. restore hello.c from the index\n \n If you want to check out all C source files out of the\n index, you can say\n \n $ git checkout -- '*.c'\n \n Note the quotes around *.c. The file hello.c will also be\n checked out, even though it is no longer in the working\n tree, because the file globbing is used to match entries in\n the index (not in the working tree by the shell).\n\nwhich is achieved through plus-continuation of each paragraph, to make\nit all part of the block under item \"1.\" in the numbered list.\n\nBut after your patch, it's:\n\n 1. The following sequence checks out the master branch,\n reverts the Makefile to two revisions back, deletes hello.c\n by mistake, and gets it back from the index.\n \n $ git checkout master (1)\n $ git checkout master~2 Makefile (2)\n $ rm -f hello.c\n $ git checkout hello.c (3)\n \n 1. switch branch\n 2. take a file out of another commit\n 3. restore hello.c from the index\n \n If you want to check out all C source files out of\n the index, you can say\n \n $ git checkout -- '*.c'\n \n Note the quotes around *.c. The file\n hello.c will also be checked out, even\n though it is no longer in the working\n tree, because the file globbing is used\n to match entries in the index (not in the\n working tree by the shell).\n\nThe plus-continuation seems to get attached to the final item of the\ncallout list, rather than to the surrounding block. I tried one or two\nthings to disambiguate this (like opening a new block around the\nexample+callout section), but couldn't get any reasonable output. It's\nalso weird that it keeps indenting each block further (I'd expect \"Note\nthe quotes\" to be in line with \"If you want to\").\n\nThis is perhaps a bug in asciidoc itself. Building with USE_ASCIIDOCTOR\nis mostly good, though it seems to drop the newline between the callout\nlist and the next paragraph, which our custom one has:\n\n 1. The following sequence checks out the master branch,\n reverts the Makefile to two revisions back, deletes hello.c\n by mistake, and gets it back from the index.\n \n $ git checkout master (1)\n $ git checkout master~2 Makefile (2)\n $ rm -f hello.c\n $ git checkout hello.c (3)\n \n 1. switch branch\n 2. take a file out of another commit\n 3. restore hello.c from the index\n If you want to check out all C source files out of the\n index, you can say\n \n $ git checkout -- '*.c'\n \n Note the quotes around *.c. The file hello.c will also be\n checked out, even though it is no longer in the working\n tree, because the file globbing is used to match entries in\n the index (not in the working tree by the shell).\n\nIt's probably still worth moving forward with your patch, as I think it\ntakes us in the direction we want long-term (and which builds with\nasciidoctor are already using). But we may want to pair it with a patch\nto work around the issue with git-checkout.1 using asciidoc to avoid\nregressing that section. It may require re-wording or re-organizing to\nwork around the bug.\n\n-Peff\n"},{"id":"475584","messageId":"20230418044142.GA130759@coredump.intra.peff.net","threadId":"59613","inReplyTo":"20230418040034.GC60552@coredump.intra.peff.net","subject":"Re: [PATCH] doc: remove custom callouts format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-04-18T04:41:42Z","receivedAt":"2023-04-18T04:41:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 18, 2023 at 12:00:34AM -0400, Jeff King wrote:\n\n> It's probably still worth moving forward with your patch, as I think it\n> takes us in the direction we want long-term (and which builds with\n> asciidoctor are already using). But we may want to pair it with a patch\n> to work around the issue with git-checkout.1 using asciidoc to avoid\n> regressing that section. It may require re-wording or re-organizing to\n> work around the bug.\n\nJust to clarify my comment on asciidoctor: since our Makefile will put\nits output through docbook, too, it actually is using the xsl you're\nremoving here (though I'm unclear on why its output looks good in\ngeneral even before your patch).\n\nBut the problem with git-checkout.1 in particular is that the XML\ngenerated by asciidoc does not close the at the right spot\n(it sticks the continuation onto the calloutlist), whereas asciidoctor\ndoes it right (it closes the calloutlist after item 3).\n\nI had tried putting an open block around the whole example and callout\nlist before, which asciidoc doesn't like. But doing it just around the\ncallout list, like so:\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex 6bb32ab460..bd83a6e5d2 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -530,9 +530,11 @@ $ rm -f hello.c\n $ git checkout hello.c <3>\n ------------\n +\n+--\n <1> switch branch\n <2> take a file out of another commit\n <3> restore `hello.c` from the index\n+--\n +\n If you want to check out _all_ C source files out of the index,\n you can say\n\nseems to improve things. The generated XML looks sensible, and the\noutput looks like asciidoctor's, though unfortunately both omit the\nblank line between the end of the callout list and the next paragraph.\nThat might be good enough to live with.\n\n-Peff\n"},{"id":"475586","messageId":"643e2aeae0515_217cdb29473@chronos.notmuch","threadId":"59613","inReplyTo":"20230418040034.GC60552@coredump.intra.peff.net","subject":"Re: [PATCH] doc: remove custom callouts format","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-04-18T05:30:18Z","receivedAt":"2023-04-18T05:30:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Mon, Apr 17, 2023 at 07:18:28PM -0600, Felipe Contreras wrote:\n> \n> > What's worse: the format of the upstream callouts is much nicer than our\n> > hacked version.\n> > \n> > Compare this:\n> > \n> > $ git diff (1)\n> > $ git diff --cached (2)\n> > $ git diff HEAD (3)\n> > \n> > 1. Changes in the working tree not yet staged for the next\n> > commit.\n> > 2. Changes between the index and your last commit; what you\n> > would be committing if you run git commit without -a\n> > option.\n> > 3. Changes in the working tree since your last commit; what\n> > you would be committing if you run git commit -a\n> > \n> > To this:\n> > \n> > $ git diff (1)\n> > $ git diff --cached (2)\n> > $ git diff HEAD (3)\n> > \n> > 1. Changes in the working tree not yet staged for the next commit.\n> > 2. Changes between the index and your last commit; what you would\n> > be committing if you run git commit without -a option.\n> > 3. Changes in the working tree since your last commit; what you\n> > would be committing if you run git commit -a\n> \n> You don't say which is which, so I'm hoping that the top one is the new\n> output. :) And running doc-diff shows that it is. Good.\n> \n> It does look like at least one spot is made worse, though. In\n> git-checkout, we have now:\n> \n> 1. The following sequence checks out the master branch,\n> reverts the Makefile to two revisions back, deletes hello.c\n> by mistake, and gets it back from the index.\n> \n> $ git checkout master (1)\n> $ git checkout master~2 Makefile (2)\n> $ rm -f hello.c\n> $ git checkout hello.c (3)\n> \n> 1. switch branch\n> 2. take a file out of another commit\n> 3. restore hello.c from the index\n> \n> If you want to check out all C source files out of the\n> index, you can say\n> \n> $ git checkout -- '*.c'\n> \n> Note the quotes around *.c. The file hello.c will also be\n> checked out, even though it is no longer in the working\n> tree, because the file globbing is used to match entries in\n> the index (not in the working tree by the shell).\n> \n> which is achieved through plus-continuation of each paragraph, to make\n> it all part of the block under item \"1.\" in the numbered list.\n> \n> But after your patch, it's:\n> \n> 1. The following sequence checks out the master branch,\n> reverts the Makefile to two revisions back, deletes hello.c\n> by mistake, and gets it back from the index.\n> \n> $ git checkout master (1)\n> $ git checkout master~2 Makefile (2)\n> $ rm -f hello.c\n> $ git checkout hello.c (3)\n> \n> 1. switch branch\n> 2. take a file out of another commit\n> 3. restore hello.c from the index\n> \n> If you want to check out all C source files out of\n> the index, you can say\n> \n> $ git checkout -- '*.c'\n> \n> Note the quotes around *.c. The file\n> hello.c will also be checked out, even\n> though it is no longer in the working\n> tree, because the file globbing is used\n> to match entries in the index (not in the\n> working tree by the shell).\n\nThat's because git's usage of asciidoc leaves much to be desired.\n\nThe `+` character is used as a list continuation, how is the doc parser\nsupposed to know that we want to continue the 1. list item, or the 3. callout?\n\nThat's why instead of:\n\n 1. foo\n +\n bar\n +\n roo\n\nIt's better (and easier) to use an open block.\n\n 1. foo\n +\n --\n bar\n\n roo\n --\n\nThis is what the documentation of asciidoctor recommends, and it even has an\nexample to attack a block to a grandparent list item.\n\nIf we do this then the parser has no trouble understanding what we are trying\nto do:\n\n --- a/Documentation/git-checkout.txt\n +++ b/Documentation/git-checkout.txt\n @@ -523,36 +523,37 @@ EXAMPLES\n the `Makefile` to two revisions back, deletes `hello.c` by\n mistake, and gets it back from the index.\n +\n +--\n ------------\n $ git checkout master <1>\n $ git checkout master~2 Makefile <2>\n $ rm -f hello.c\n $ git checkout hello.c <3>\n ------------\n -+\n <1> switch branch\n <2> take a file out of another commit\n <3> restore `hello.c` from the index\n -+\n +\n If you want to check out _all_ C source files out of the index,\n you can say\n -+\n +\n ------------\n $ git checkout -- '*.c'\n ------------\n -+\n +\n Note the quotes around `*.c`. The file `hello.c` will also be\n checked out, even though it is no longer in the working tree,\n because the file globbing is used to match entries in the index\n (not in the working tree by the shell).\n -+\n +\n If you have an unfortunate branch that is named `hello.c`, this\n step would be confused as an instruction to switch to that branch.\n You should instead write:\n -+\n +\n ------------\n $ git checkout -- hello.c\n ------------\n +--\n \n . After working in the wrong branch, switching to the correct\n branch would be done using:\n\n\n> This is perhaps a bug in asciidoc itself.\n\nIn my opinion it's a bug in our usage of asciidoc. It happened to look OK by\naccident.\n\n> Building with USE_ASCIIDOCTOR is mostly good, though it seems to drop the\n> newline between the callout list and the next paragraph, which our custom one\n> has:\n\nThat's a DocBook Stylesheets problem, if you apply the patch above to use an\nopen block, then you get the same output with both asciidoc and asciidoctor.\n\nI don't see why we insist on creating such complex list items though.\n\nCreating a subsection is much clearer for everyone: the reader, the writer, and\nthe parser:\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex 6bb32ab460..2d16651d1f 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -519,84 +519,89 @@ to checkout these paths out of the index.\n EXAMPLES\n --------\n \n-. The following sequence checks out the `master` branch, reverts\n- the `Makefile` to two revisions back, deletes `hello.c` by\n- mistake, and gets it back from the index.\n-+\n+=== 1\n+\n+The following sequence checks out the `master` branch, reverts\n+the `Makefile` to two revisions back, deletes `hello.c` by\n+mistake, and gets it back from the index.\n+\n ------------\n $ git checkout master <1>\n $ git checkout master~2 Makefile <2>\n $ rm -f hello.c\n $ git checkout hello.c <3>\n ------------\n-+\n <1> switch branch\n <2> take a file out of another commit\n <3> restore `hello.c` from the index\n-+\n+\n If you want to check out _all_ C source files out of the index,\n you can say\n-+\n+\n ------------\n $ git checkout -- '*.c'\n ------------\n-+\n+\n Note the quotes around `*.c`. The file `hello.c` will also be\n checked out, even though it is no longer in the working tree,\n because the file globbing is used to match entries in the index\n (not in the working tree by the shell).\n-+\n+\n If you have an unfortunate branch that is named `hello.c`, this\n step would be confused as an instruction to switch to that branch.\n You should instead write:\n-+\n+\n ------------\n $ git checkout -- hello.c\n ------------\n \n> It's probably still worth moving forward with your patch, as I think it\n> takes us in the direction we want long-term (and which builds with\n> asciidoctor are already using). But we may want to pair it with a patch\n> to work around the issue with git-checkout.1 using asciidoc to avoid\n> regressing that section. It may require re-wording or re-organizing to\n> work around the bug.\n\nI can add that patch depending what we want:\n\nOpen block:\n\n 1. foo\n +\n --\n bar\n\n roo\n --\n\nOr subsection:\n\n === 1\n\n foo\n\n bar\n\n roo\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"475590","messageId":"20230418061713.GA169940@coredump.intra.peff.net","threadId":"59613","inReplyTo":"643e2aeae0515_217cdb29473@chronos.notmuch","subject":"Re: [PATCH] doc: remove custom callouts format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-04-18T06:17:13Z","receivedAt":"2023-04-18T06:17:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 17, 2023 at 11:30:18PM -0600, Felipe Contreras wrote:\n\n> If we do this then the parser has no trouble understanding what we are trying\n> to do:\n> \n> --- a/Documentation/git-checkout.txt\n> +++ b/Documentation/git-checkout.txt\n> @@ -523,36 +523,37 @@ EXAMPLES\n> the `Makefile` to two revisions back, deletes `hello.c` by\n> mistake, and gets it back from the index.\n> +\n> +--\n> ------------\n> $ git checkout master <1>\n> $ git checkout master~2 Makefile <2>\n> $ rm -f hello.c\n> $ git checkout hello.c <3>\n> ------------\n> -+\n> <1> switch branch\n> <2> take a file out of another commit\n> <3> restore `hello.c` from the index\n> -+\n\nAh, that makes sense. I tried something like this, but asciidoc was\nunhappy with the \"+\" continuation between the example and the callout\ninside the block (which makes sense as there is no \"list\" to continue\nwithin that block).\n\nJust putting the example and its callouts in a block is sufficient, but\nI agree that putting all of the \"The following sequence...\" list item's\ncontent into a single block makes the source easier to read.\n\n> I don't see why we insist on creating such complex list items though.\n> \n> Creating a subsection is much clearer for everyone: the reader, the writer, and\n> the parser:\n\nUnless the subsection has a meaningful title, the formatting of the\nresult looks a bit odd to me:\n\n EXAMPLES\n 1\n The following sequence checks out the master branch, reverts\n the Makefile to two revisions back, deletes hello.c by mistake,\n and gets it back from the index.\n\nas opposed to:\n\n 1. The following sequence checks out the master branch,\n reverts the Makefile to two revisions back, deletes hello.c\n by mistake, and gets it back from the index.\n\nIf each example had a short section title, it would make more sense.\n\nAt any rate, IMHO it is probably best to start with purely syntactic\nfixes that don't change the output, as that is uncontroversial and\naddresses the issue with your original patch (which is otherwise making\nmost spots look nicer). And then any conversation about sections versus\nlists can proceed independently.\n\n> > It's probably still worth moving forward with your patch, as I think it\n> > takes us in the direction we want long-term (and which builds with\n> > asciidoctor are already using). But we may want to pair it with a patch\n> > to work around the issue with git-checkout.1 using asciidoc to avoid\n> > regressing that section. It may require re-wording or re-organizing to\n> > work around the bug.\n> \n> I can add that patch depending what we want:\n> \n> Open block:\n> \n> 1. foo\n> +\n> --\n> bar\n> \n> roo\n> --\n> \n> Or subsection:\n> \n> === 1\n> \n> foo\n> \n> bar\n> \n> roo\n\nSo I'd prefer the open block.\n\n-Peff\n"},{"id":"475595","messageId":"643e43824a220_21b043294f8@chronos.notmuch","threadId":"59613","inReplyTo":"20230418061713.GA169940@coredump.intra.peff.net","subject":"Re: [PATCH] doc: remove custom callouts format","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-04-18T07:15:14Z","receivedAt":"2023-04-18T07:15:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Mon, Apr 17, 2023 at 11:30:18PM -0600, Felipe Contreras wrote:\n> \n> > If we do this then the parser has no trouble understanding what we are trying\n> > to do:\n> > \n> > --- a/Documentation/git-checkout.txt\n> > +++ b/Documentation/git-checkout.txt\n> > @@ -523,36 +523,37 @@ EXAMPLES\n> > the `Makefile` to two revisions back, deletes `hello.c` by\n> > mistake, and gets it back from the index.\n> > +\n> > +--\n> > ------------\n> > $ git checkout master <1>\n> > $ git checkout master~2 Makefile <2>\n> > $ rm -f hello.c\n> > $ git checkout hello.c <3>\n> > ------------\n> > -+\n> > <1> switch branch\n> > <2> take a file out of another commit\n> > <3> restore `hello.c` from the index\n> > -+\n> \n> Ah, that makes sense. I tried something like this, but asciidoc was\n> unhappy with the \"+\" continuation between the example and the callout\n> inside the block (which makes sense as there is no \"list\" to continue\n> within that block).\n> \n> Just putting the example and its callouts in a block is sufficient, but\n> I agree that putting all of the \"The following sequence...\" list item's\n> content into a single block makes the source easier to read.\n> \n> > I don't see why we insist on creating such complex list items though.\n> > \n> > Creating a subsection is much clearer for everyone: the reader, the writer, and\n> > the parser:\n> \n> Unless the subsection has a meaningful title, the formatting of the\n> result looks a bit odd to me:\n> \n> EXAMPLES\n> 1\n> The following sequence checks out the master branch, reverts\n> the Makefile to two revisions back, deletes hello.c by mistake,\n> and gets it back from the index.\n> \n> as opposed to:\n> \n> 1. The following sequence checks out the master branch,\n> reverts the Makefile to two revisions back, deletes hello.c\n> by mistake, and gets it back from the index.\n\nTo me it's the exact opposite: it's odd that the whole example is indented,\nwhen it's part of the main content.\n\nI prefer the example to be indented at the same level as all the other text.\n\nAlso, because I use manpage color trick, the subsection header \"1\" is rendered\nin red, while the list number is not. It's odd that even the callout numbers\ninside the example are highlighted in red, but not the number of the example.\n\n> If each example had a short section title, it would make more sense.\n\nI don't particularly mind, but if you think that's a requirement, then why not\nadd that?\n\nI sent a patch series doing precisely t hat.\n\n> At any rate, IMHO it is probably best to start with purely syntactic\n> fixes that don't change the output, as that is uncontroversial and\n> addresses the issue with your original patch (which is otherwise making\n> most spots look nicer). And then any conversation about sections versus\n> lists can proceed independently.\n\nBut the syntactic fixes changes the output.\n\nHave you looked at the HTML output with asciidoc-py? It has the same\nindentation problem you spotted in the manpages.\n\nI don't see it in git-scm.com, but I presume that documentation there is\ngenerated with asciidoctor.\n\n> > > It's probably still worth moving forward with your patch, as I think it\n> > > takes us in the direction we want long-term (and which builds with\n> > > asciidoctor are already using). But we may want to pair it with a patch\n> > > to work around the issue with git-checkout.1 using asciidoc to avoid\n> > > regressing that section. It may require re-wording or re-organizing to\n> > > work around the bug.\n> > \n> > I can add that patch depending what we want:\n> > \n> > Open block:\n> > \n> > 1. foo\n> > +\n> > --\n> > bar\n> > \n> > roo\n> > --\n> > \n> > Or subsection:\n> > \n> > === 1\n> > \n> > foo\n> > \n> > bar\n> > \n> > roo\n> \n> So I'd prefer the open block.\n\nWhat if I add a proper title?\n\n === 2. Merge\n\nAnother advantage is that we can link directly to the subsection in HTML:\n\n git-checkout.html#_2_merge\n\nIt's not something that's probably going to be used in practice, but to me it\nmakes total semantic sense to have big chunks of prose in a section of its own.\n\nHaving a huge list item on the other hand does not make sense, it would be like\nhaving a list item that spans more than one page of a book.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"475596","messageId":"643e45d9222ae_21b04329432@chronos.notmuch","threadId":"59613","inReplyTo":"20230418044142.GA130759@coredump.intra.peff.net","subject":"Re: [PATCH] doc: remove custom callouts format","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-04-18T07:25:13Z","receivedAt":"2023-04-18T07:25:42Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Tue, Apr 18, 2023 at 12:00:34AM -0400, Jeff King wrote:\n> \n> > It's probably still worth moving forward with your patch, as I think it\n> > takes us in the direction we want long-term (and which builds with\n> > asciidoctor are already using). But we may want to pair it with a patch\n> > to work around the issue with git-checkout.1 using asciidoc to avoid\n> > regressing that section. It may require re-wording or re-organizing to\n> > work around the bug.\n> \n> Just to clarify my comment on asciidoctor: since our Makefile will put\n> its output through docbook, too, it actually is using the xsl you're\n> removing here (though I'm unclear on why its output looks good in\n> general even before your patch).\n\nIt's probably a quirk of the git hack for callouts, but that's specific to\nmanpages.\n\nHave you looked at the HTML output generated by asciidoc-py? The badly indented\noutput suggests this issue has nothing to do with my patch.\n\n-- \nFelipe Contreras\n"},{"id":"475599","messageId":"20230418090310.GA414708@coredump.intra.peff.net","threadId":"59613","inReplyTo":"643e43824a220_21b043294f8@chronos.notmuch","subject":"Re: [PATCH] doc: remove custom callouts format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-04-18T09:03:10Z","receivedAt":"2023-04-18T09:03:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 18, 2023 at 01:15:14AM -0600, Felipe Contreras wrote:\n\n> Have you looked at the HTML output with asciidoc-py? It has the same\n> indentation problem you spotted in the manpages.\n> \n> I don't see it in git-scm.com, but I presume that documentation there is\n> generated with asciidoctor.\n\nI hadn't looked at it, but yeah, I see it has the same issue. That makes\nsense, since the XML output from asciidoc was wrong (and our xslt was\npapering over it for the manpage build).\n\nAnd yes, the pages no git-scm.com are built with asciidoctor.\n\n> > So I'd prefer the open block.\n> \n> What if I add a proper title?\n> \n> === 2. Merge\n\nFrom the perspective of somebody skimming through the examples, that\ndoesn't seem to help much. But I'm not sure if there _is_ a succinct and\ndescriptive title for each of those examples (or at least I could not\neasily come up with one, which is why I didn't offer a suggestion).\n\n> It's not something that's probably going to be used in practice, but to me it\n> makes total semantic sense to have big chunks of prose in a section of its own.\n> \n> Having a huge list item on the other hand does not make sense, it would be like\n> having a list item that spans more than one page of a book.\n\nWe may have to agree to disagree on that. But this is exactly why I\nsuggested doing the syntactic fix first, rather than reorganizing. Once\nthe fix is done, then there can be a separate discussion on reorganizing\n(which, frankly, I don't really have much interest in either way; I gave\nmy opinion and I don't have anything else to say).\n\n-Peff\n"},{"id":"475602","messageId":"643e67d71c0ae_21cdec29495@chronos.notmuch","threadId":"59613","inReplyTo":"20230418090310.GA414708@coredump.intra.peff.net","subject":"Re: [PATCH] doc: remove custom callouts format","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-04-18T09:50:15Z","receivedAt":"2023-04-18T09:50:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Tue, Apr 18, 2023 at 01:15:14AM -0600, Felipe Contreras wrote:\n> \n> > Have you looked at the HTML output with asciidoc-py? It has the same\n> > indentation problem you spotted in the manpages.\n> > \n> > I don't see it in git-scm.com, but I presume that documentation there is\n> > generated with asciidoctor.\n> \n> I hadn't looked at it, but yeah, I see it has the same issue. That makes\n> sense, since the XML output from asciidoc was wrong (and our xslt was\n> papering over it for the manpage build).\n\nYeah, so the problem is already there and had nothing to do with my patch.\n\nI sent a separate patch series to fix that _separate_ problem.\n\n> > > So I'd prefer the open block.\n> > \n> > What if I add a proper title?\n> > \n> > === 2. Merge\n> \n> From the perspective of somebody skimming through the examples, that\n> doesn't seem to help much.\n\nI think it helps that the examples are not indented to a level that no main\nprose in the manpage is indented at.\n\n> > It's not something that's probably going to be used in practice, but to me it\n> > makes total semantic sense to have big chunks of prose in a section of its own.\n> > \n> > Having a huge list item on the other hand does not make sense, it would be like\n> > having a list item that spans more than one page of a book.\n> \n> We may have to agree to disagree on that.\n\nDo we though?\n\nDo you actually think the output of this command with a huge list item makes\nsemantic sense?\n\n (\n printf '1. '\n for x in $(seq 1 10000); do\n printf 'foo '\n done\n echo\n\n printf '2. bar\\n'\n ) | asciidoctor - -o test.html\n\n\n> But this is exactly why I suggested doing the syntactic fix first, rather\n> than reorganizing.\n\nThe syntactic fix is--first of all--orthogonal to my patch.\n\nAnd secondly: causes another glitch. So it doesn't seem like much of a fix,\nmore like a workaround in which we trade a big glitch for a small glitch.\n\n> Once the fix is done, then there can be a separate discussion on reorganizing\n> (which, frankly, I don't really have much interest in either way; I gave my\n> opinion and I don't have anything else to say).\n\nI'm not sure it's a fix, but more relevantly: I'm not sure what that has to do\nwith my patch.\n\nThe fix for how we use asciidoc in git-checkout.txt can be implemented in a\ntotally separate patch series that has nothing to do with $subject.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}