| Message ID | 97c686dd7ba1bbd1c0be6f7f61a3a033adf8adb6.1613590761.git.martin.agren@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 83171ede2229b71ef46cd4d2f800c1eef27cc511 |
| Headers | show |
| Series | fix some doc rendering issues since v2.30.0 | expand |
On Wed, Feb 17, 2021 at 1:21 PM Martin Ågren <martin.agren@gmail.com> wrote: > > When we write `<name>`s with the "s" tucked on to the closing backtick, > we end up rendering the backticks literally. Rephrase this sentence > slightly to render this as monospace. That seems fine, but one question (diff trimmed way down to make it clearer I hope): > + contain an equals sign to avoid ambiguity with <name> containing > + to avoid ambiguity with `<name>` containing one. One replacement drops the backquotes entirely. The other keeps them. Surely these two shouldn't be *different*...? Chris
On Wed, Feb 17, 2021 at 08:56:05PM +0100, Martin Ågren wrote: > When we write `<name>`s with the "s" tucked on to the closing backtick, > we end up rendering the backticks literally. Rephrase this sentence > slightly to render this as monospace. > > Signed-off-by: Martin Ågren <martin.agren@gmail.com> > --- > doc-diff: > --- a/.../man/man1/git.1 > +++ b/.../man/man1/git.1 > @@ -77,8 +77,8 @@ OPTIONS > setting the value to an empty string, instead the environment > variable itself must be set to the empty string. It is an error if > the <envvar> does not exist in the environment. <envvar> may not > - contain an equals sign to avoid ambiguity with `<name>`s which > - contain one. > + contain an equals sign to avoid ambiguity with <name> containing > + one. Over here you're also dropping the backticks, while... > This is useful for cases where you want to pass transitory > configuration options to git, but are doing so on OS’s where other > Documentation/git.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/git.txt b/Documentation/git.txt > index d36e6fd482..3a9c44987f 100644 > --- a/Documentation/git.txt > +++ b/Documentation/git.txt > @@ -88,7 +88,7 @@ foo.bar= ...`) sets `foo.bar` to the empty string which `git config > empty string, instead the environment variable itself must be > set to the empty string. It is an error if the `<envvar>` does not exist > in the environment. `<envvar>` may not contain an equals sign > - to avoid ambiguity with `<name>`s which contain one. > + to avoid ambiguity with `<name>` containing one. ... here you don't. Is this on purpose? Patrick > This is useful for cases where you want to pass transitory > configuration options to git, but are doing so on OS's where > -- > 2.30.0.284.gd98b1dd5ea >
On Wed, 17 Feb 2021 at 23:47, Chris Torek <chris.torek@gmail.com> wrote: > > On Wed, Feb 17, 2021 at 1:21 PM Martin Ågren <martin.agren@gmail.com> wrote: > > > > When we write `<name>`s with the "s" tucked on to the closing backtick, > > we end up rendering the backticks literally. Rephrase this sentence > > slightly to render this as monospace. > > That seems fine, but one question (diff trimmed way down to > make it clearer I hope): > > > + contain an equals sign to avoid ambiguity with <name> containing > > > + to avoid ambiguity with `<name>` containing one. > > One replacement drops the backquotes entirely. The other keeps > them. Surely these two shouldn't be *different*...? I included the output of our "doc-diff" script below the double-dash line. The patch applies just as fine anyway, but I did wonder if it would trip up human readers. :-/ Quoting my original, slightly less trimmed: On Wed, 17 Feb 2021 at 20:56, Martin Ågren <martin.agren@gmail.com> wrote: > doc-diff: > --- a/.../man/man1/git.1 > +++ b/.../man/man1/git.1 > - contain an equals sign to avoid ambiguity with `<name>`s which > - contain one. > + contain an equals sign to avoid ambiguity with <name> containing > + one. So that's how the rendering is changed. From "oops, we rendered the backticks literally" to "we no longer do". (It's not clear from the doc-diff that they're rendered monospace/bold, but at least this is no longer obviously broken.) (Note the extra indentation of all of that. This is where one might place some commentary that one don't want to burden the commit message with, but which also isn't part of the actual diff. Now that this looks very much like a diff, I can see how it's confusing.) And that's because of this change to the actual sources: > diff --git a/Documentation/git.txt b/Documentation/git.txt > index d36e6fd482..3a9c44987f 100644 > --- a/Documentation/git.txt > +++ b/Documentation/git.txt > - to avoid ambiguity with `<name>`s which contain one. > + to avoid ambiguity with `<name>` containing one. I hope that clarifies it? It's a bit unfortunate that the misrendering is so similar to the source in the txt file. But I guess that's still better than some of those misrenderings where some cogwheel slips and everything spins out of control through the rest of the paragraph. Martin
On Thu, 18 Feb 2021 at 07:27, Patrick Steinhardt <ps@pks.im> wrote: > > On Wed, Feb 17, 2021 at 08:56:05PM +0100, Martin Ågren wrote: > > When we write `<name>`s with the "s" tucked on to the closing backtick, > > we end up rendering the backticks literally. Rephrase this sentence > > slightly to render this as monospace. > > > > Signed-off-by: Martin Ågren <martin.agren@gmail.com> > > --- > > doc-diff: > > --- a/.../man/man1/git.1 > > +++ b/.../man/man1/git.1 > > @@ -77,8 +77,8 @@ OPTIONS > > setting the value to an empty string, instead the environment > > variable itself must be set to the empty string. It is an error if > > the <envvar> does not exist in the environment. <envvar> may not > > - contain an equals sign to avoid ambiguity with `<name>`s which > > - contain one. > > + contain an equals sign to avoid ambiguity with <name> containing > > + one. > > Over here you're also dropping the backticks, while... > > > This is useful for cases where you want to pass transitory > > configuration options to git, but are doing so on OS’s where other > > Documentation/git.txt | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/git.txt b/Documentation/git.txt > > index d36e6fd482..3a9c44987f 100644 > > --- a/Documentation/git.txt > > +++ b/Documentation/git.txt > > @@ -88,7 +88,7 @@ foo.bar= ...`) sets `foo.bar` to the empty string which `git config > > empty string, instead the environment variable itself must be > > set to the empty string. It is an error if the `<envvar>` does not exist > > in the environment. `<envvar>` may not contain an equals sign > > - to avoid ambiguity with `<name>`s which contain one. > > + to avoid ambiguity with `<name>` containing one. > > ... here you don't. Is this on purpose? Your mail crossed with my response to Chris, who had the same question. I'd post a link to lore.kernel.org, but it seems my response hasn't reached it yet. The short answer is the first diff is an indented diff of the rendered manpages (our "doc-diff" script), whereas the second diff is the actual, to-be-applied diff. I thought it would be helpful to include the doc-diff, but it seems it just created more confusion than it avoided. I'll try to avoid that. :-) Thanks Martin
Martin Ågren <martin.agren@gmail.com> writes: > When we write `<name>`s with the "s" tucked on to the closing backtick, > we end up rendering the backticks literally. Rephrase this sentence > slightly to render this as monospace. > > Signed-off-by: Martin Ågren <martin.agren@gmail.com> > --- > doc-diff: > --- a/.../man/man1/git.1 > +++ b/.../man/man1/git.1 > @@ -77,8 +77,8 @@ OPTIONS > setting the value to an empty string, instead the environment > variable itself must be set to the empty string. It is an error if > the <envvar> does not exist in the environment. <envvar> may not > - contain an equals sign to avoid ambiguity with `<name>`s which > - contain one. > + contain an equals sign to avoid ambiguity with <name> containing > + one. > > This is useful for cases where you want to pass transitory > configuration options to git, but are doing so on OS’s where other > Documentation/git.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/git.txt b/Documentation/git.txt > index d36e6fd482..3a9c44987f 100644 > --- a/Documentation/git.txt > +++ b/Documentation/git.txt > @@ -88,7 +88,7 @@ foo.bar= ...`) sets `foo.bar` to the empty string which `git config > empty string, instead the environment variable itself must be > set to the empty string. It is an error if the `<envvar>` does not exist > in the environment. `<envvar>` may not contain an equals sign > - to avoid ambiguity with `<name>`s which contain one. > + to avoid ambiguity with `<name>` containing one. > + > This is useful for cases where you want to pass transitory > configuration options to git, but are doing so on OS's where The "two" diffs may indeed be misleading. The patch only changes one source and the "supporting material" is not something that we need to use on the source file---it is only showing the change in the output. I did appreciate the inclusion of doc-diff myself, but it seems that it confused Chris and Patrick. I doubt that it is an improvement to omit doc-diff. We may want to find a way to make it easier for the readers to tell which part is the patch to be applied and which part is not in similar changes we discuss on the list in the future, and apparently, a one column indentation alone was not quite sufficient in this case. Replacing "doc-diff:" label and patch header lines up to the hunk header with a prose to explain what the intended diff is may have helped, e.g. ... slightly to render this as monospace. Signed-off-by: A U Thor <au@thor.xz> --- The rendered output changes like the following excerpt from doc-diff output. .... <envvar> may not - contain an equals sign to avoid ambiguity with `<name>`s which - contain one. + contain an equals sign to avoid ambiguity with <name> containing + one. You can see that backquotes are gone, even though you unfortunately cannot see how <name> is typeset (it is in monospace--trust me ;-). Documentation/git.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.... index .... --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ ...
On Thu, 18 Feb 2021 at 19:51, Junio C Hamano <gitster@pobox.com> wrote: > > The "two" diffs may indeed be misleading. > > The patch only changes one source and the "supporting material" is > not something that we need to use on the source file---it is only > showing the change in the output. > > I did appreciate the inclusion of doc-diff myself, but it seems that > it confused Chris and Patrick. I doubt that it is an improvement to > omit doc-diff. We may want to find a way to make it easier for the > readers to tell which part is the patch to be applied and which part > is not in similar changes we discuss on the list in the future, and > apparently, a one column indentation alone was not quite sufficient > in this case. Replacing "doc-diff:" label and patch header lines up > to the hunk header with a prose to explain what the intended diff is > may have helped, e.g. Glad to know you found the included doc-diff useful. Thanks for the suggestion on how to present it. Next time I'll try wrapping the doc-diff in English prose to make it stand out more. The "doc-diff:" I used now was definitely a bit too subtle. Thanks Martin
diff --git a/Documentation/git.txt b/Documentation/git.txt index d36e6fd482..3a9c44987f 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -88,7 +88,7 @@ foo.bar= ...`) sets `foo.bar` to the empty string which `git config empty string, instead the environment variable itself must be set to the empty string. It is an error if the `<envvar>` does not exist in the environment. `<envvar>` may not contain an equals sign - to avoid ambiguity with `<name>`s which contain one. + to avoid ambiguity with `<name>` containing one. + This is useful for cases where you want to pass transitory configuration options to git, but are doing so on OS's where
When we write `<name>`s with the "s" tucked on to the closing backtick, we end up rendering the backticks literally. Rephrase this sentence slightly to render this as monospace. Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- doc-diff: --- a/.../man/man1/git.1 +++ b/.../man/man1/git.1 @@ -77,8 +77,8 @@ OPTIONS setting the value to an empty string, instead the environment variable itself must be set to the empty string. It is an error if the <envvar> does not exist in the environment. <envvar> may not - contain an equals sign to avoid ambiguity with `<name>`s which - contain one. + contain an equals sign to avoid ambiguity with <name> containing + one. This is useful for cases where you want to pass transitory configuration options to git, but are doing so on OS’s where other Documentation/git.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)