| Message ID | 20230418070048.2209469-2-felipe.contreras@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f8bc75a55e877e3f3e71c36eae6e1ee8710e5a84 |
| Headers | show |
| Series | doc: git-checkout: trivial style improvements | expand |
Felipe Contreras <felipe.contreras@gmail.com> writes: > The callouts are directly tied to the listing above, remove spaces to > make it clear they are one and the same. > > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> > --- > Documentation/git-checkout.txt | 4 ---- > 1 file changed, 4 deletions(-) I tried to format git-checkout.1 and git-checkout.html from HEAD and HEAD^ after applying this step, with asciidoc and asciidoctor, and did not see any difference that came from this patch. Am I correct to understand that this patch is done purely for the benefit of human readers, and not for formatting machinery? It may be subjective for those who read the source if it is easier to read with or without inter-paragraph spaces, but in any case, the resulting source material now look the same way between two hunks, and consistency is good. Queued. Thanks. > diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt > index 6bb32ab460..8ddeec63dd 100644 > --- a/Documentation/git-checkout.txt > +++ b/Documentation/git-checkout.txt > @@ -483,14 +483,11 @@ $ git checkout -b foo # or "git switch -c foo" <1> > $ git branch foo <2> > $ git tag foo <3> > ------------ > - > <1> creates a new branch `foo`, which refers to commit `f`, and then > updates `HEAD` to refer to branch `foo`. In other words, we'll no longer > be in detached `HEAD` state after this command. > - > <2> similarly creates a new branch `foo`, which refers to commit `f`, > but leaves `HEAD` detached. > - > <3> creates a new tag `foo`, which refers to commit `f`, > leaving `HEAD` detached. > > @@ -529,7 +526,6 @@ $ 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
Junio C Hamano wrote: > Felipe Contreras <felipe.contreras@gmail.com> writes: > > > The callouts are directly tied to the listing above, remove spaces to > > make it clear they are one and the same. > > > > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> > > --- > > Documentation/git-checkout.txt | 4 ---- > > 1 file changed, 4 deletions(-) > > I tried to format git-checkout.1 and git-checkout.html from HEAD and > HEAD^ after applying this step, with asciidoc and asciidoctor, and > did not see any difference that came from this patch. Am I correct > to understand that this patch is done purely for the benefit of > human readers, and not for formatting machinery? No, it's for the formatting machinery. The fact that both asciidoc and asciidoctor happen to understand our quircky formatting in this particualr situation doesn't mean it isn't quirky. In this particular case the parsers do understand what we are trying to do, because we just just pepper list continuations (`+`) everywhere and it happens to work. This works: 1. item + ---- line 1 <1> ---- + <1> callout 1 But if we used an open block instead (which is more propper), this does not: 1. item + -- ---- line 1 <1> ---- + <1> callout 1 -- This discrepancy confused Jeff in [1]. [1] https://lore.kernel.org/git/20230418061713.GA169940@coredump.intra.peff.net/ > It may be subjective for those who read the source if it is easier > to read with or without inter-paragraph spaces, but in any case, the > resulting source material now look the same way between two hunks, > and consistency is good. That is an added benefit. It's simply a good practice to follow the format asciidoctor documentation: which doesn't contain spaces, doesn't require adding list continations, it's easier to interpret by the parsers, and works inside open blocks.
Felipe Contreras <felipe.contreras@gmail.com> writes: > No, it's for the formatting machinery. > > The fact that both asciidoc and asciidoctor happen to understand our quircky > formatting in this particualr situation doesn't mean it isn't quirky. > > In this particular case the parsers do understand what we are trying to do, > because we just just pepper list continuations (`+`) everywhere and it happens > to work. I'll stop at pointing out that the first "no" sounds much stronger than the text that tries to substantiate it, which says that the machinery works fine without the changes. > This discrepancy confused Jeff in [1]. And this is a good reason to add this change for humans. > It's simply a good practice to follow the format asciidoctor documentation: That one I would agree with 100%. Thanks.
On Mon, Apr 24, 2023 at 05:02:25PM -0700, Junio C Hamano wrote: > Felipe Contreras <felipe.contreras@gmail.com> writes: > > > No, it's for the formatting machinery. > > > > The fact that both asciidoc and asciidoctor happen to understand our quircky > > formatting in this particualr situation doesn't mean it isn't quirky. > > > > In this particular case the parsers do understand what we are trying to do, > > because we just just pepper list continuations (`+`) everywhere and it happens > > to work. > > I'll stop at pointing out that the first "no" sounds much stronger > than the text that tries to substantiate it, which says that the > machinery works fine without the changes. > > > This discrepancy confused Jeff in [1]. > > And this is a good reason to add this change for humans. Since I'm being used as the example, I'd like to point that I think this is somewhat tangential to what actually confused me there. What confused me in that earlier message was that having "+" as the continuation between a code-block and its call-out list is odd, since "+" is about continuing a list item. It happens to work because we're in a larger list item, but it breaks when you put the two of them in their own block (which is the part that got me). Using just a blank line between the code block and the call-out list (instead of the "+") works for asciidoc (it is happy to keep the two together) but not asciidoctor (it ends the outer ordered list before starting the callout list). So the second hunk in the patch, to drop the extra continuation between the code block and the callout, makes perfect sense to me. The first hunk seems less obviously good to me. We're not part of a list, so there's no continuation. We might say that it is good to always stick the callout list directly adjacent to the associated code block, since it does matter in other cases. But dropping the blank lines between the paragraph-sized callout blocks makes the source less readable, and empty lines between list elements are a pretty normal thing in asciidoc. That said, I don't feel _too_ strongly about it, so I'm OK with the patch as-is. -Peff
Junio C Hamano wrote: > Felipe Contreras <felipe.contreras@gmail.com> writes: > > > No, it's for the formatting machinery. > > > > The fact that both asciidoc and asciidoctor happen to understand our quircky > > formatting in this particualr situation doesn't mean it isn't quirky. > > > > In this particular case the parsers do understand what we are trying to do, > > because we just just pepper list continuations (`+`) everywhere and it happens > > to work. > > I'll stop at pointing out that the first "no" sounds much stronger > than the text that tries to substantiate it, which says that the > machinery works fine without the changes. The question was if this patch was done *purely* for the benefit of human readers: > > > Am I correct to understand that this patch is done purely for the benefit > > > of human readers, and not for formatting machinery? Even if the conclusion was that it was done primarily for the benefit of human readers and secondarily for machine parsers (it shouldn't), that's still a "no". > > This discrepancy confused Jeff in [1]. > > And this is a good reason to add this change for humans. > > > It's simply a good practice to follow the format asciidoctor documentation: > > That one I would agree with 100%. --- To draw a parallel with a situation the git dev community is much more familiar with: we can think of asciidoc as the shell language. AsiiDoc is trying to be a specified language [1], just like POSIX shell. The fact that some syntax happens to work on all the *current* shells does not mean it is specified, and it does not mean that it must work on future shells. Similarly, the fact some asciidoc syntax happens to work on the current transformers doesn't mean it will work on all future ones. Moreover, transformers are not the only consideration, as text editors trying to parse the same text also matter. Not to mention blurry lines like human-like-machines like LLMs helping humans with the code, and machine-like-humans because apparently the way I read the asciidoc code is different than my fellow humans (I'm thinking of how a parser could interpret it). Either way, if the question was about *my* motivation for the patch, it was just consistency, which helps both humans and machines. Cheers. [1] https://asciidoc-wg.eclipse.org/
Jeff King wrote: > On Mon, Apr 24, 2023 at 05:02:25PM -0700, Junio C Hamano wrote: > > > Felipe Contreras <felipe.contreras@gmail.com> writes: > > > > > No, it's for the formatting machinery. > > > > > > The fact that both asciidoc and asciidoctor happen to understand our quircky > > > formatting in this particualr situation doesn't mean it isn't quirky. > > > > > > In this particular case the parsers do understand what we are trying to do, > > > because we just just pepper list continuations (`+`) everywhere and it happens > > > to work. > > > > I'll stop at pointing out that the first "no" sounds much stronger > > than the text that tries to substantiate it, which says that the > > machinery works fine without the changes. > > > > > This discrepancy confused Jeff in [1]. > > > > And this is a good reason to add this change for humans. > > Since I'm being used as the example, I'd like to point that I think this > is somewhat tangential to what actually confused me there. > > What confused me in that earlier message was that having "+" as the > continuation between a code-block and its call-out list is odd, since > "+" is about continuing a list item. Indeed. > It happens to work because we're in a larger list item, No, there's only one list item, but yes, it works when the callout is inside one. The `+` is continuing the list item the callout list is part of, it's not part of the callout. > but it breaks when you put the two of them in their own block (which is the > part that got me). You only need to put one, but when you do, the `+` doesn't belong there as it's not part of the callout. > Using just a blank line between the code block and the call-out list > (instead of the "+") works for asciidoc (it is happy to keep the two > together) but not asciidoctor (it ends the outer ordered list before > starting the callout list). I don't know what you mean. These are three paragraphs: foo bar roo These are two paragraphs inside a list item with continuations: 1. foo + bar + roo These are three paragraphs inside a list item with an open block: 1. foo + -- bar roo -- If you are inside an open block, you don't need the list continuations (`+`). Therefore a callout inside an open block inside a list item does not need continuations: 1. foo + -- ---- line 1 <1> line 2 <2> ---- <1> callout 1 <2> callout 2 -- Just like a callout outside a list item: ---- line 1 <1> line 2 <2> ---- <1> callout 1 <2> callout 2 It's only a callout inside a list item with no open block that needs continuations, just like any other paragraph: 1. foo + ---- line 1 <1> line 2 <2> ---- + <1> callout 1 + <2> callout 2 > So the second hunk in the patch, to drop the extra continuation between > the code block and the callout, makes perfect sense to me. > > The first hunk seems less obviously good to me. I don't see why: it's exactly the same change: removing an unnecessary space. Except that space is represented with a `+` inside a list item. > We might say that it is good to always > stick the callout list directly adjacent to the associated code block, > since it does matter in other cases. I'd say that is good. > But dropping the blank lines between the paragraph-sized callout blocks makes > the source less readable, That is a value judgement. It may be less readable to you, I disagree. I would expect the subsequent callout blocks as close to the listing block they refer to as possible. > and empty lines between list elements are a pretty normal thing in asciidoc. Usually, yeah when these list elements are independent, but callout elements are not independent. To me this is perfectly readable: This is a simple example in sh: ---- #!/bin/sh <1> echo 'hello world' <2> ---- <1> Shebang <2> "echo" prints This is a simple example in Ruby: ---- #!/usr/bin/env ruby <1> puts 'hello world' <2> ---- <1> Standard shebang <2> "puts" puts a string Put spaces on the callouts, and it's the exact opposite: This is a simple example in sh: ---- #!/bin/sh <1> echo 'hello world' <2> ---- <1> Shebang <2> "echo" prints This is a simple example in Ruby: ---- #!/usr/bin/env ruby <1> puts 'hello world' <2> ---- <1> Standard shebang <2> "puts" puts a string Cheers.
On Thu, Apr 27, 2023 at 04:09:32PM -0600, Felipe Contreras wrote: > > Using just a blank line between the code block and the call-out list > > (instead of the "+") works for asciidoc (it is happy to keep the two > > together) but not asciidoctor (it ends the outer ordered list before > > starting the callout list). > > I don't know what you mean. For reference, I meant this: diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 6bb32ab460..ed32497290 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -529,7 +529,7 @@ $ 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 which asciidoc renders the same, but asciidoctor is not. Your patch is now in master removing the line entirely, though, so the point is moot. -Peff
Jeff King wrote: > On Thu, Apr 27, 2023 at 04:09:32PM -0600, Felipe Contreras wrote: > > > > Using just a blank line between the code block and the call-out list > > > (instead of the "+") works for asciidoc (it is happy to keep the two > > > together) but not asciidoctor (it ends the outer ordered list before > > > starting the callout list). > > > > I don't know what you mean. > > For reference, I meant this: > > diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt > index 6bb32ab460..ed32497290 100644 > --- a/Documentation/git-checkout.txt > +++ b/Documentation/git-checkout.txt > @@ -529,7 +529,7 @@ $ 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 > > which asciidoc renders the same, but asciidoctor is not. I see. I would say the code is ambiguous, so it's not surprising that a parser interprets it as an end of a list element.
Jeff King <peff@peff.net> writes: >> > Using just a blank line between the code block and the call-out list >> > (instead of the "+") works for asciidoc (it is happy to keep the two >> > together) but not asciidoctor (it ends the outer ordered list before >> > starting the callout list). >> ... > $ git checkout hello.c <3> > ------------ > -+ > + > <1> switch branch > <2> take a file out of another commit > <3> restore `hello.c` from the index > > which asciidoc renders the same, but asciidoctor is not. Yet another random annoying differences that explains why we wrote it that way in the first place X-<. If it has been removed to make it not to matter, that was lucky of us ;-) Thanks.
Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > >> > Using just a blank line between the code block and the call-out list > >> > (instead of the "+") works for asciidoc (it is happy to keep the two > >> > together) but not asciidoctor (it ends the outer ordered list before > >> > starting the callout list). > >> ... > > $ git checkout hello.cgit checkout hello.c <3> > > ------------ > > -+ > > + > > <1> switch branch > > <2> take a file out of another commit > > <3> restore `hello.c` from the index > > > > which asciidoc renders the same, but asciidoctor is not. > > Yet another random annoying differences that explains why we wrote > it that way in the first place X-<. I don't think so. This is the sequence of events: 1. 1e2ccd3abc (Documentation: more examples., 2005-12-12) 2. 1be0659efc (checkout: merge local modifications while switching branches., 2006-01-12) 3. 48aeecdcc1 (Fix up remaining man pages that use asciidoc "callouts"., 2006-04-28) In 1 "callouts" were added inside the listing itself, so they were not actually callouts, the space was added just for formatting. In 2 other examples were added, which transformed the original example to a list element, and everything was peppered with `+`. Finally in 3 the code was transformed to actual AsciiDoc callouts, which meant that the original space that previously was protected inside a listing block would not be protected anymore, so it could either be 1) removed or 2) converted to `+`. In the thread that added this `+` [1] I see no discussion about the alternatives. At no point did anyone consider using a newline inside a list element, because spaces inside list elements don't work, so the actual two choices were: A: ---- line 1 <1> ---- <1> callout 1 B: ---- line 1 <1> ---- + <1> callout 1 And there never was any actual reason given as to why B was "chosen". I think the reality is most of the AsciiDoc code doesn't really have any consideration as to what is the actual AsciiDoc syntax, it's there because it seems to work, and that's it. So I think the real reason is that a newline was there before, and newlines stop list elements. It couldn't have possibly been because a newline didn't work in asciidoctor, because asciidoctor would not have been created for another 6 years. > If it has been removed to make it not to matter, that was lucky of us ;-) Only if following the actual syntax of a language is considered "luck", in the same way that it's "lucky" that following the C specification makes C code work on most C compilers. [1] https://lore.kernel.org/git/BAYC1-PASMTP028F5C1C39F7EF6A6EEB95AEB20@CEZ.ICE/
Jeff King <peff@peff.net> writes: > The first hunk seems less obviously good to me. We're not part of a > list, so there's no continuation. We might say that it is good to always > stick the callout list directly adjacent to the associated code block, > since it does matter in other cases. But dropping the blank lines > between the paragraph-sized callout blocks makes the source less > readable, and empty lines between list elements are a pretty normal > thing in asciidoc. > > That said, I don't feel _too_ strongly about it, so I'm OK with the > patch as-is. I agree with the conclusion. The first hunk may be iffy (in the sense that "it is not cut-and-dried better than the original"), but the change makes the postimage of that hunk look similar to the postimage of the second hunk, and adopting it as our local rule to alwys stick the callout list to the associated block gives us more consistent look, that is good, too.
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 6bb32ab460..8ddeec63dd 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -483,14 +483,11 @@ $ git checkout -b foo # or "git switch -c foo" <1> $ git branch foo <2> $ git tag foo <3> ------------ - <1> creates a new branch `foo`, which refers to commit `f`, and then updates `HEAD` to refer to branch `foo`. In other words, we'll no longer be in detached `HEAD` state after this command. - <2> similarly creates a new branch `foo`, which refers to commit `f`, but leaves `HEAD` detached. - <3> creates a new tag `foo`, which refers to commit `f`, leaving `HEAD` detached. @@ -529,7 +526,6 @@ $ 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
The callouts are directly tied to the listing above, remove spaces to make it clear they are one and the same. Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> --- Documentation/git-checkout.txt | 4 ---- 1 file changed, 4 deletions(-)