Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: git diff reformatting #1769

Closed
wants to merge 5 commits into from
Closed

Conversation

jnavila
Copy link

@jnavila jnavila commented Aug 4, 2024

This is the continuation of the editing of the manpages to reflect the new formatting rules.

Changes since V3:

  • rework the description about -1, --base,...

cc: Johannes Sixt j6t@kdbg.org
cc: Patrick Steinhardt ps@pks.im

@jnavila jnavila force-pushed the git_diff_new branch 3 times, most recently from 4c615e5 to acb5bdd Compare August 4, 2024 19:59
@jnavila
Copy link
Author

jnavila commented Aug 4, 2024

/submit

Copy link

gitgitgadget bot commented Aug 4, 2024

Submitted as pull.1769.git.1722801936.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1769/jnavila/git_diff_new-v1

To fetch this version to local tag pr-1769/jnavila/git_diff_new-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1769/jnavila/git_diff_new-v1

@@ -14,7 +14,7 @@ You can customize the creation of patch text via the
`GIT_EXTERNAL_DIFF` and the `GIT_DIFF_OPTS` environment variables
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Sixt wrote (reply to this):

Am 04.08.24 um 22:05 schrieb Jean-Noël Avila via GitGitGadget:
> @@ -134,17 +135,18 @@ or like this (when the `--cc` option is used):
>  
>  2.   It is followed by one or more extended header lines
>       (this example shows a merge with two parents):
> -
> -       index <hash>,<hash>..<hash>
> -       mode <mode>,<mode>..<mode>
> -       new file mode <mode>
> -       deleted file mode <mode>,<mode>
>  +
> -The `mode <mode>,<mode>..<mode>` line appears only if at least one of
> -the <mode> is different from the rest. Extended headers with
> +[synopsis]
> +index <hash>,<hash>`..`<hash>
> +mode <mode>,<mode>`..`<mode>
> +new file mode <mode>
> +deleted file mode <mode>,<mode>
> ++
> +The `mode` __<mode>__++,++__<mode>__++..++__<mode>__ line appears only if at least one of
> +the _<mode>_ is different from the rest. Extended headers with

I've a strong aversion to the formatting that this series applies,
because it introduces many (IMHO) unnecessary punctuation that
vandalizes the perfectly readable plain text. And this hunk now shows
where it goes too far. These lines under the new [synopsis] header just
aren't syopsis; they are comamnd output. The updated version abuses a
semantic token to achieve syntactic highlighting.

To me this series looks too much like "we must adapt to the tool" when
the correct stance should be "the tool must adapt to us". If the tool
(one of asciidoc and asciidoctor, I presume) does not cooperate well
with out documents, then it is the tool that must be changed, not our
documents.

I understand that some compromises are needed, but with this extent of
changes we give in to a sub-par tool too far.

Just my 2c.

-- Hannes

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Johannes Sixt <j6t@kdbg.org> writes:

> I've a strong aversion to the formatting that this series applies,
> because it introduces many (IMHO) unnecessary punctuation that
> vandalizes the perfectly readable plain text. And this hunk now shows
> where it goes too far. These lines under the new [synopsis] header just
> aren't syopsis; they are comamnd output. The updated version abuses a
> semantic token to achieve syntactic highlighting.
>
> To me this series looks too much like "we must adapt to the tool" when
> the correct stance should be "the tool must adapt to us". If the tool
> (one of asciidoc and asciidoctor, I presume) does not cooperate well
> with out documents, then it is the tool that must be changed, not our
> documents.
>
> I understand that some compromises are needed, but with this extent of
> changes we give in to a sub-par tool too far.

Thanks for placing this into words a lot better than how I could
have done.  I share the same feeling.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Jean-Noël AVILA wrote (reply to this):

On Monday, 5 August 2024 07:53:19 CEST Johannes Sixt wrote:
> Am 04.08.24 um 22:05 schrieb Jean-Noël Avila via GitGitGadget:
> > @@ -134,17 +135,18 @@ or like this (when the `--cc` option is used):
> >  
> >  2.   It is followed by one or more extended header lines
> >       (this example shows a merge with two parents):
> > -
> > -       index <hash>,<hash>..<hash>
> > -       mode <mode>,<mode>..<mode>
> > -       new file mode <mode>
> > -       deleted file mode <mode>,<mode>
> >  +
> > -The `mode <mode>,<mode>..<mode>` line appears only if at least one of
> > -the <mode> is different from the rest. Extended headers with
> > +[synopsis]
> > +index <hash>,<hash>`..`<hash>
> > +mode <mode>,<mode>`..`<mode>
> > +new file mode <mode>
> > +deleted file mode <mode>,<mode>
> > ++
> > +The `mode` __<mode>__++,++__<mode>__++..++__<mode>__ line appears only if 
at least one of
> > +the _<mode>_ is different from the rest. Extended headers with
> 
> I've a strong aversion to the formatting that this series applies,
> because it introduces many (IMHO) unnecessary punctuation that
> vandalizes the perfectly readable plain text. And this hunk now shows
> where it goes too far. These lines under the new [synopsis] header just
> aren't syopsis; they are comamnd output. The updated version abuses a
> semantic token to achieve syntactic highlighting.
> 
> To me this series looks too much like "we must adapt to the tool" when
> the correct stance should be "the tool must adapt to us". If the tool
> (one of asciidoc and asciidoctor, I presume) does not cooperate well
> with out documents, then it is the tool that must be changed, not our
> documents.
> 
> I understand that some compromises are needed, but with this extent of
> changes we give in to a sub-par tool too far.
> 
> Just my 2c.
> 
> -- Hannes
> 
> 

Hello,

I was half expecting this kind of reactions. First there are some remarks on 
your remarks.
 
 * You think the markup is unnecessary. To me, it is critical in order to make 
the documentation output a little more meaningful. My experience as a 
translator is that there's a great lack of consistency in the vocabulary, the 
grammar styles, the typesetting and the cross-references of the pages. On top 
of that, they are clearly not user-oriented. Overall, the joke about how 
cryptic the man-pages of Git are is not coming from nowhere. There's no one to 
blame: we are all developers doing our best, but we are not technical writers 
dedicated to this project.
 * The fact that the source of the pages should be "perfectly readable" is a 
moot argument. Fair enough, it is not the objective to make it impossible to 
understand, but in the end, this is not what is consumed: these pages are 
compiled into other formats where the markup has been translated into styling. 
I suspect some writers are not thinking when quoting text, that this is not a 
quotation but an inline formatting command. But this is markup, and sometimes, 
it cannot  be removed of the way.
 * I converted the lines to synopsis because there are placeholders in them. 
These lines are presented as an example but they are neither. This is another 
example of communication impedance,  where the presented text is not what it 
is described as, and requires from the reader to interpret what the writer was 
thinking and forgot to make clear to a newcomer.


As for the "need to adapt to the tool", for block formatting, I tried to get 
something bearable (the synopis style); I'd really like something similar for 
inline formatting but my asciidoc/asciidoctor Fu requires an upgrade in order 
to make it happen. As it seems to be too epidermic, I'll try to cook something 
anyway and keep being open to any proposition.

In the mean time, a proper editor setup (syntax highlighting and fontification 
, two panels with one showing the compiled output) can alleviate your pain. 

JN

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Jean-Noël AVILA wrote (reply to this):

On Monday, 5 August 2024 18:08:07 CEST Junio C Hamano wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
> 
> > I've a strong aversion to the formatting that this series applies,
> > because it introduces many (IMHO) unnecessary punctuation that
> > vandalizes the perfectly readable plain text. And this hunk now shows
> > where it goes too far. These lines under the new [synopsis] header just
> > aren't syopsis; they are comamnd output. The updated version abuses a
> > semantic token to achieve syntactic highlighting.
> >
> > To me this series looks too much like "we must adapt to the tool" when
> > the correct stance should be "the tool must adapt to us". If the tool
> > (one of asciidoc and asciidoctor, I presume) does not cooperate well
> > with out documents, then it is the tool that must be changed, not our
> > documents.
> >
> > I understand that some compromises are needed, but with this extent of
> > changes we give in to a sub-par tool too far.
> 
> Thanks for placing this into words a lot better than how I could
> have done.  I share the same feeling.
> 

I'm working on a form of macro that would work almost the same way as the 
synopsis paragraph. You would have some markup, but it would be surrounding 
the text to typeset: 

s:["--ignore-matching-lines=<regex>"]

In this snippet the macro part (which is recognized by a regex) is  s:[ ]
The inside part is managed the same. If you need additional markup, it is 
possible:

s:["<commit1>`...`<commit2>"] to have the three dots rendered as <code>, 
because they are part of the tokens.

Square brackets are possible inside the double-quotes:
s:["--ignore-submodules[=<when>]"]

Is this something that wouldn't repel you?

Best regards,

JN

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Sixt wrote (reply to this):

Am 07.08.24 um 22:43 schrieb Jean-Noël AVILA:
> On Monday, 5 August 2024 18:08:07 CEST Junio C Hamano wrote:
>> Johannes Sixt <j6t@kdbg.org> writes:
>>
>>> I've a strong aversion to the formatting that this series applies,
>>> because it introduces many (IMHO) unnecessary punctuation that
>>> vandalizes the perfectly readable plain text. And this hunk now shows
>>> where it goes too far. These lines under the new [synopsis] header just
>>> aren't syopsis; they are comamnd output. The updated version abuses a
>>> semantic token to achieve syntactic highlighting.
>>>
>>> To me this series looks too much like "we must adapt to the tool" when
>>> the correct stance should be "the tool must adapt to us". If the tool
>>> (one of asciidoc and asciidoctor, I presume) does not cooperate well
>>> with out documents, then it is the tool that must be changed, not our
>>> documents.
>>>
>>> I understand that some compromises are needed, but with this extent of
>>> changes we give in to a sub-par tool too far.
>>
>> Thanks for placing this into words a lot better than how I could
>> have done.  I share the same feeling.
>>
> 
> I'm working on a form of macro that would work almost the same way as the 
> synopsis paragraph. You would have some markup, but it would be surrounding 
> the text to typeset: 
> 
> s:["--ignore-matching-lines=<regex>"]
> 
> In this snippet the macro part (which is recognized by a regex) is  s:[ ]
> The inside part is managed the same. If you need additional markup, it is 
> possible:
> 
> s:["<commit1>`...`<commit2>"] to have the three dots rendered as <code>, 
> because they are part of the tokens.
> 
> Square brackets are possible inside the double-quotes:
> s:["--ignore-submodules[=<when>]"]
> 
> Is this something that wouldn't repel you?

You argued elsewhere in this thread:

>  * The fact that the source of the pages should be "perfectly readable" is a 
> moot argument. Fair enough, it is not the objective to make it impossible to 
> understand, but in the end, this is not what is consumed: these pages are 
> compiled into other formats where the markup has been translated into styling. 

I buy this argument, in particular, since not even I read the plain text
files, but use the rendered version.

I would like tone down my harsh opposition to mild opposition. IMO, it
should still be easy to *write* the documentation. It should not be
necessary that authors remember to use macros all over the place.

And I still think that we should not introduce macros just to please all
renderers. Let's just pick the one renderer that can do the job best. If
it means that some distribution cannot render the documentation
perfectly themselves (Debian? I don't know), they can always use the
pre-rendered version that Junio kindly produces.

-- Hannes

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Johannes Sixt <j6t@kdbg.org> writes:

>> Square brackets are possible inside the double-quotes:
>> s:["--ignore-submodules[=<when>]"]
>> 
>> Is this something that wouldn't repel you?
>
> You argued elsewhere in this thread:
>
>>  * The fact that the source of the pages should be "perfectly readable" is a 
>> moot argument. Fair enough, it is not the objective to make it impossible to 
>> understand, but in the end, this is not what is consumed: these pages are 
>> compiled into other formats where the markup has been translated into styling. 
>
> I buy this argument, in particular, since not even I read the plain text
> files, but use the rendered version.

FWIW, I do read the plain text files, and rarely if ever use the
HTML version, except when checking the effect of changes to the
mark-up.

> I would like tone down my harsh opposition to mild opposition. IMO, it
> should still be easy to *write* the documentation. It should not be
> necessary that authors remember to use macros all over the place.

Yeah, s:[...] does repel me, but also I do not think it is sensible
to claim that we confortably edit the "source" form that we find it
hard to read.

> And I still think that we should not introduce macros just to please all
> renderers. Let's just pick the one renderer that can do the job best. If
> it means that some distribution cannot render the documentation
> perfectly themselves (Debian? I don't know), they can always use the
> pre-rendered version that Junio kindly produces.

What Junio uses "Debian? I don't know" that cannot render the
documentation ;-)?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Jean-Noël AVILA wrote (reply to this):

On Monday, 12 August 2024 08:35:39 CEST Johannes Sixt wrote:
> Am 07.08.24 um 22:43 schrieb Jean-Noël AVILA:
> 
> I would like tone down my harsh opposition to mild opposition. IMO, it
> should still be easy to *write* the documentation. It should not be
> necessary that authors remember to use macros all over the place.

The purpose of this series is to clarify the formatting rules for keywords and 
placeholders, and to uniformly apply them, so that the readers can relate the 
meaning of what they are reading with the visual cues in the text.  The more 
uniform the typesetting, the less surprised the reader, the smaller the 
communication impedance.

This requirement makes the documentation *less* easy to write, for sure.
It is no question of forcing authors to use the formatting macro everywhere. 
As explained in the Guildelines V3 of the series, the macro is introduced in 
order to remove the most hairy forms where manually doing the formatting would 
lead to difficult to read/write sequences. I bet most writers will remember and 
use the s macro when they want to typeset something like 
--ignore-submodules[=<when>]

As an added benefit, we can also simplify some existing parts, for instance see 
the ones in urls.txt.

> 
> And I still think that we should not introduce macros just to please all
> renderers. Let's just pick the one renderer that can do the job best. If
> it means that some distribution cannot render the documentation
> perfectly themselves (Debian? I don't know), they can always use the
> pre-rendered version that Junio kindly produces.

I do not understand how the renderer could solve the issue of typesetting the 
"good part" in the place of the writers. The macro is there to mechanize the 
typesetting of selected parts, but it is up to the writers to define what is a 
keyword and what is a placeholder in their prose. Please note also that using 
proper placeholder differentiating and typesetting should have the side-effect 
of making the prose lighter, by removing the need to express which placeholder 
we are talking about.

To me, Asciidoc strikes a good balance for a tool that makes it easy to write 
simple things and not too complicated to write more complex ones. It is also 
customizable for specific needs, which is handy for our use case.  I am not 
aware of an existing renderer that would do the job really best. What do you 
have in mind?

JN





Copy link

gitgitgadget bot commented Aug 5, 2024

User Johannes Sixt <j6t@kdbg.org> has been added to the cc: list.

@@ -8,13 +8,13 @@ git-diff - Show changes between commits, commit and working tree, etc

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Patrick Steinhardt wrote (reply to this):

On Sun, Aug 04, 2024 at 08:05:32PM +0000, Jean-Noël Avila via GitGitGadget wrote:
> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
> 

Nit: please provide a summary of what adaptations you made. It would
certainly help the reviewer to learn about the recently-introduced
`[synopsis]` style and why/since when we use backticks and underscores
for formatting.

The same also applies to subsequent commits, providing some pointers
would certainly help an unknowing reviewer such as myself.

> @@ -225,11 +225,12 @@ CONFIGURATION
>  
>  include::includes/cmd-config-section-all.txt[]
>  
> +:git-diff: 1
>  include::config/diff.txt[]
>  
>  SEE ALSO
>  --------
> -diff(1),
> +`diff`(1),

This one looks curious to me. Why is this item formatted differently
than the next three? I would have expected it to be formatted as
something like linkgit:git-diff[1] instead of `diff`(1)`.

Patrick

>  linkgit:git-difftool[1],
>  linkgit:git-log[1],
>  linkgit:gitdiffcore[7],
> -- 
> gitgitgadget
> 
> 

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Jean-Noël AVILA wrote (reply to this):

On Monday, 5 August 2024 11:11:07 CEST Patrick Steinhardt wrote:
> On Sun, Aug 04, 2024 at 08:05:32PM +0000, Jean-Noël Avila via GitGitGadget 
wrote:
> > From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
> > 
> 
> Nit: please provide a summary of what adaptations you made. It would
> certainly help the reviewer to learn about the recently-introduced
> `[synopsis]` style and why/since when we use backticks and underscores
> for formatting.
> 
> The same also applies to subsequent commits, providing some pointers
> would certainly help an unknowing reviewer such as myself.
> 

Thanks for the remark. The context is effectively missing, so I'll restate it.

> > @@ -225,11 +225,12 @@ CONFIGURATION
> >  
> >  include::includes/cmd-config-section-all.txt[]
> >  
> > +:git-diff: 1
> >  include::config/diff.txt[]
> >  
> >  SEE ALSO
> >  --------
> > -diff(1),
> > +`diff`(1),
> 
> This one looks curious to me. Why is this item formatted differently
> than the next three? I would have expected it to be formatted as
> something like linkgit:git-diff[1] instead of `diff`(1)`.
> 

Here we are referring to the 'diff' command, not git-diff. That is why we don't 
use the linkgit macro (which is used to generate cross links for html output).

Still, the general format of a reference to a man-page is to put the command 
name in bold, which our filters get by backquoting. 

> Patrick
> 
> >  linkgit:git-difftool[1],
> >  linkgit:git-log[1],
> >  linkgit:gitdiffcore[7],
> 

Thanks

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Patrick Steinhardt wrote (reply to this):

On Mon, Aug 05, 2024 at 08:51:21PM +0200, Jean-Noël AVILA wrote:
> On Monday, 5 August 2024 11:11:07 CEST Patrick Steinhardt wrote:
> > On Sun, Aug 04, 2024 at 08:05:32PM +0000, Jean-Noël Avila via GitGitGadget 
> wrote:
> > > @@ -225,11 +225,12 @@ CONFIGURATION
> > >  
> > >  include::includes/cmd-config-section-all.txt[]
> > >  
> > > +:git-diff: 1
> > >  include::config/diff.txt[]
> > >  
> > >  SEE ALSO
> > >  --------
> > > -diff(1),
> > > +`diff`(1),
> > 
> > This one looks curious to me. Why is this item formatted differently
> > than the next three? I would have expected it to be formatted as
> > something like linkgit:git-diff[1] instead of `diff`(1)`.
> > 
> 
> Here we are referring to the 'diff' command, not git-diff. That is why we don't 
> use the linkgit macro (which is used to generate cross links for html output).
> 
> Still, the general format of a reference to a man-page is to put the command 
> name in bold, which our filters get by backquoting. 

Oh, that makes sense of course. I totally forgot that there's a world
outside of Git. Thanks for the clarification!

Patrick

Copy link

gitgitgadget bot commented Aug 5, 2024

User Patrick Steinhardt <ps@pks.im> has been added to the cc: list.

@jnavila jnavila force-pushed the git_diff_new branch 3 times, most recently from 9b47dce to 17a2f02 Compare November 10, 2024 18:53
@jnavila
Copy link
Author

jnavila commented Nov 10, 2024

/submit

1 similar comment
@jnavila
Copy link
Author

jnavila commented Nov 10, 2024

/submit

@jnavila
Copy link
Author

jnavila commented Nov 11, 2024

Hello @dscho

It seems I can no longer submit this PR, the gitgitgadget bot does not react to the submit commentaries.

@dscho
Copy link
Member

dscho commented Nov 11, 2024

Hmm. It all started yesterday.

Copy link

gitgitgadget bot commented Nov 11, 2024

Submitted as pull.1769.v2.git.1731343985.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1769/jnavila/git_diff_new-v2

To fetch this version to local tag pr-1769/jnavila/git_diff_new-v2:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1769/jnavila/git_diff_new-v2

@dscho
Copy link
Member

dscho commented Nov 11, 2024

@jnavila sorry about the problems. I still do not quite know what's happening, I do suspect that it has something to do with a recent Git update, though. Will keep having an eye on it, but I will much appreciate a ping in case of problems!

@jnavila
Copy link
Author

jnavila commented Nov 11, 2024

The log seems to indicate some kind of conflict while trying to run Git in parallel, which is quite strange, given that this part is just supposed to clone the project.

@dscho
Copy link
Member

dscho commented Nov 11, 2024

The log seems to indicate some kind of conflict while trying to run Git in parallel, which is quite strange, given that this part is just supposed to clone the project.

Indeed. In the first failing build the find call did not find any .lock files, even. So it must be some race within the git reset --hard call itself. I added GIT_TRACE2_PERF tracing just in case, hoping that it will help us by identifying threads and/or processes that might try to lock that file simultaneously.

@@ -8,13 +8,13 @@ git-diff - Show changes between commits, commit and working tree, etc

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Jean-Noël Avila via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +[synopsis]
> +git diff [<options>] [<commit>] [--] [<path>...]
> +git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]
> +git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]
> +git diff [<options>] <commit>...<commit> [--] [<path>...]
> +git diff [<options>] <blob> <blob>
> +git diff [<options>] --no-index [--] <path> <path>

Again, not having to worry about `mark-up` _<rules>_ in SYNOPSIS section
is very nice.

You may already have explained the rules elsewhere, but please
help me refresh my memory with some explanation.

> -'git diff' [<options>] [--] [<path>...]::
> +`git diff [<options>] [--] [<path>...]`::

Here, we just say `everything in literal, including placeholders`,
which is very pleasant for us writers.

> --1 --base::
> --2 --ours::
> --3 --theirs::
> +`-1` `--base`::
> +`-2` `--ours`::
> +`-3` `--theirs`::

Why aren't these `-1 --base` and instead mark up individual tokens?

> -<path>...::
> -	The <paths> parameters, when given, are used to limit
> +_<path>_...::

This has to do the _italics_ for placeholders, unlike the full
command line examples we saw earlier?

Where does this difference come from?

Thanks.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Jean-Noël Avila wrote (reply to this):

Le 12/11/2024 à 01:48, Junio C Hamano a écrit :
> "Jean-Noël Avila via GitGitGadget" <gitgitgadget@gmail.com> writes:

> 
> You may already have explained the rules elsewhere, but please
> help me refresh my memory with some explanation.
> 
>> -'git diff' [<options>] [--] [<path>...]::
>> +`git diff [<options>] [--] [<path>...]`::
> 
> Here, we just say `everything in literal, including placeholders`,
> which is very pleasant for us writers.


With the new document processor extension, the back tick quotes have
become smarter and they behave basically like an inline synopsis
section. Here, this means that the line will be formatted roughly as
follows:

`git diff` [_<options>_] [`--`] [_<path>_...]

All the keywords are literal, the placeholders are emphasized, and the
syntactic signs ('[', ']', '...') are left without formatting.

The general rule of thumb for the writer is: if it's a singled
placeholder then quote it with underscores, otherwise use back ticks
elsewhere.

> 
>> --1 --base::
>> --2 --ours::
>> --3 --theirs::
>> +`-1` `--base`::
>> +`-2` `--ours`::
>> +`-3` `--theirs`::
> 
> Why aren't these `-1 --base` and instead mark up individual tokens?
> 

Here, it is quite awkward, because we are mixing alternate spellings of
the same option (`-1` and `--base` have the same meaning) with the fact
that these options are meant to be alternatives. The latter meaning is
not what is usually conveyed in the lists of options, which blurs the
following explanation.

To clarify, from what I understand, it would be better to fully spell
out the way these options are used by using the synopsis syntax:

`(-1|--base) | (-2|--ours) | (-3|--theirs)`::

Is it how it works?

>> -<path>...::
>> -	The <paths> parameters, when given, are used to limit
>> +_<path>_...::
> 
> This has to do the _italics_ for placeholders, unlike the full
> command line examples we saw earlier?
> 
> Where does this difference come from?

Well, according to the rule of thumb above, the whole segment should
have been quoted in back ticks. This is a mistake and must be fixed for
consistency.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Jean-Noël Avila <jn.avila@free.fr> writes:

> With the new document processor extension, the back tick quotes have
> become smarter and they behave basically like an inline synopsis
> section. Here, this means that the line will be formatted roughly as
> follows:
>
> `git diff` [_<options>_] [`--`] [_<path>_...]

Ahh, yes, it is the key magic how your "enclosing the whole line"
works.  It's been so long since we adopted the topic that laid the
groundwork that I forgot ;-)

Again, it is very pleasant for us writers to be able to just do so.

>>> +`-1` `--base`::
>>> +`-2` `--ours`::
>>> +`-3` `--theirs`::
>> 
>> Why aren't these `-1 --base` and instead mark up individual tokens?
>> 
>
> Here, it is quite awkward, because we are mixing alternate spellings of
> the same option (`-1` and `--base` have the same meaning) with the fact
> that these options are meant to be alternatives. The latter meaning is
> not what is usually conveyed in the lists of options, which blurs the
> following explanation.
>
> To clarify, from what I understand, it would be better to fully spell
> out the way these options are used by using the synopsis syntax:
>
> `(-1|--base) | (-2|--ours) | (-3|--theirs)`::
>
> Is it how it works?

Yeah, that may be a more sensible rewrite (regardless of this
"better mark-up" topic).

Thanks.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Sixt wrote (reply to this):

Am 12.11.24 um 10:13 schrieb Junio C Hamano:
> Jean-Noël Avila <jn.avila@free.fr> writes:
>> To clarify, from what I understand, it would be better to fully spell
>> out the way these options are used by using the synopsis syntax:
>>
>> `(-1|--base) | (-2|--ours) | (-3|--theirs)`::
>>
>> Is it how it works?
> 
> Yeah, that may be a more sensible rewrite (regardless of this
> "better mark-up" topic).

I would disagree. IMHO, it is not necessary to copy this sort of
nerdfulness (the syntax annotations) from the synopsis to the
description section.

Does

`-1`, `--base`::
`-2`, `--ours`::
`-3`, `--theirs`::

not work? (I think we agree that grouping synonyms should be done and
not all tokens moved onto a flat line.)

-- Hannes

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Johannes Sixt <j6t@kdbg.org> writes:

> Does
>
> `-1`, `--base`::
> `-2`, `--ours`::
> `-3`, `--theirs`::
>
> not work? (I think we agree that grouping synonyms should be done and
> not all tokens moved onto a flat line.)

With the current

    -1 --base::
    -2 --ours::
    -3 --theirs::
	body text

these are coalesced into a single header and get rendered as

    -1 --base, -2 --ours, -3 --theirs
	body text

which reasonably shows that 1 and base belong to the same family
that is different from 2 and ours, etc.  With an explicit comma
in between 1 and base, would we end up with

    -1, --base, -2, --ours, -3, --theirs

which may be worse than what we have currently?

Thanks.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Sixt wrote (reply to this):

Am 13.11.24 um 00:01 schrieb Junio C Hamano:
> With an explicit comma
> in between 1 and base, would we end up with
> 
>     -1, --base, -2, --ours, -3, --theirs
> 
> which may be worse than what we have currently?

Agreed, that would indeed be worse.

-- Hannes

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Jean-Noël Avila wrote (reply to this):

Le 13/11/2024 à 00:01, Junio C Hamano a écrit :
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> Does
>>
>> `-1`, `--base`::
>> `-2`, `--ours`::
>> `-3`, `--theirs`::
>>
>> not work? (I think we agree that grouping synonyms should be done and
>> not all tokens moved onto a flat line.)
> 
> With the current
> 
>     -1 --base::
>     -2 --ours::
>     -3 --theirs::
> 	body text
> 
> these are coalesced into a single header and get rendered as
> 
>     -1 --base, -2 --ours, -3 --theirs
> 	body text
> 

When I first reviewed this part, I interpreted it as "there are 3 forms,
made of pairs of options used together", which did not make sense. That
is only after reading git-read-tree, that this explanation made sense.

> which reasonably shows that 1 and base belong to the same family
> that is different from 2 and ours, etc.  With an explicit comma
> in between 1 and base, would we end up with
> 
>     -1, --base, -2, --ours, -3, --theirs
> 
> which may be worse than what we have currently?
> 
> Thanks.

To be consistant with description of option, I think the 3 alternatives
should be split into their own items.

@@ -19,16 +19,16 @@ ifdef::git-format-patch[]
endif::git-format-patch[]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Jean-Noël Avila via GitGitGadget" <gitgitgadget@gmail.com> writes:

> --s::
> ---no-patch::
> +`-s`::
> +`--no-patch`::

These are understandable.  These dashed options that are to be given
literally are shown as `literal`.

> @@ -39,28 +39,28 @@ endif::git-format-patch[]
>  ifdef::git-log[]
>  -m::

Shouldn't this and all others like -c/--cc be also quoted as
`literal` options?

> @@ -73,33 +73,33 @@ The following formats are supported:
>  off, none::

Shouldn't this and other option values like on, first-parent, etc.,
that are literals be marked-up specially?

> --U<n>::
> ---unified=<n>::
> -	Generate diffs with <n> lines of context instead of
> +`-U<n>`::
> +`--unified=<n>`::
> +	Generate diffs with _<n>_ lines of context instead of

Understandable.  Shouldn't <n> part be italicised?

> ---output=<file>::
> +`--output=<file>`::

Likewise.

Thanks.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Jean-Noël Avila wrote (reply to this):

Le 12/11/2024 à 01:52, Junio C Hamano a écrit :
> "Jean-Noël Avila via GitGitGadget" <gitgitgadget@gmail.com> writes:
>  
>> @@ -39,28 +39,28 @@ endif::git-format-patch[]
>>  ifdef::git-log[]
>>  -m::
> 
> Shouldn't this and all others like -c/--cc be also quoted as
> `literal` options?

As stated in the commit message, only the parts of the files which
appear in git-diff man page are converted (like was done for git-clone
and git-init)

Thinking again about it, I don't find it wise, because other man pages
will be half-converted anyway, at least for the common parts of the
included files, and we are going to introduce several commits for the
same file.

So, better convert all the file in on run.

> 
>> @@ -73,33 +73,33 @@ The following formats are supported:
>>  off, none::
> 
> Shouldn't this and other option values like on, first-parent, etc.,
> that are literals be marked-up specially?

This is to be converted, and will be with the conversion of the full file.

> 
>> --U<n>::
>> ---unified=<n>::
>> -	Generate diffs with <n> lines of context instead of
>> +`-U<n>`::
>> +`--unified=<n>`::
>> +	Generate diffs with _<n>_ lines of context instead of
> 
> Understandable.  Shouldn't <n> part be italicised?

No need: the processing of back tick quotes already treats each part
according to its semantics and applies the corresponding format.


Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Jean-Noël Avila <jn.avila@free.fr> writes:

>>> +`-U<n>`::
>>> +`--unified=<n>`::
>>> +	Generate diffs with _<n>_ lines of context instead of
>> 
>> Understandable.  Shouldn't <n> part be italicised?
>
> No need: the processing of back tick quotes already treats each part
> according to its semantics and applies the corresponding format.

Yup, you reminded me of how `the backticks are much more clever
these days` thing works.  Thanks.

Copy link

gitgitgadget bot commented Nov 12, 2024

This patch series was integrated into seen via git@2934ac3.

@gitgitgadget gitgitgadget bot added the seen label Nov 12, 2024
@@ -1,25 +1,25 @@
Raw output format
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Sixt wrote (reply to this):

Am 11.11.24 um 17:53 schrieb Jean-Noël Avila via GitGitGadget:
> -The raw output format from "git-diff-index", "git-diff-tree",
> -"git-diff-files" and "git diff --raw" are very similar.
> +The raw output format from `git-diff-index`, `git-diff-tree`,
> +`git-diff-files` and `git diff --raw` are very similar.

Throughout this document we see a lot of commands with dashes `git-foo`.
Does this have any significance, or should they be corrected to the
dashless form `git foo`? It could even be a "while at it"-change as you
are touching every instance anyway.

>  
>  These commands all compare two sets of things; what is
>  compared differs:
>  
> -git-diff-index <tree-ish>::
> -        compares the <tree-ish> and the files on the filesystem.
> +`git-diff-index <tree-ish>`::
> +	compares the _<tree-ish>_ and the files on the filesystem.

Now that the backtick formats the content as in the synopsis, why is it
written _<tree-ish>_ and not `<tree-ish>` in the running text?

-- Hannes

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Johannes Sixt <j6t@kdbg.org> writes:

> Am 11.11.24 um 17:53 schrieb Jean-Noël Avila via GitGitGadget:
>> -The raw output format from "git-diff-index", "git-diff-tree",
>> -"git-diff-files" and "git diff --raw" are very similar.
>> +The raw output format from `git-diff-index`, `git-diff-tree`,
>> +`git-diff-files` and `git diff --raw` are very similar.
>
> Throughout this document we see a lot of commands with dashes `git-foo`.
> Does this have any significance, or should they be corrected to the
> dashless form `git foo`? It could even be a "while at it"-change as you
> are touching every instance anyway.

Yup, these "git-foo" are historical wart from pre Git 1.6 days.

>>  These commands all compare two sets of things; what is
>>  compared differs:
>>  
>> -git-diff-index <tree-ish>::
>> -        compares the <tree-ish> and the files on the filesystem.
>> +`git-diff-index <tree-ish>`::
>> +	compares the _<tree-ish>_ and the files on the filesystem.
>
> Now that the backtick formats the content as in the synopsis, why is it
> written _<tree-ish>_ and not `<tree-ish>` in the running text?

Perhaps that is because the body text does want to show the
placeholder in _italics_ but not as a `literal` string?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Sixt wrote (reply to this):

Am 13.11.24 um 00:03 schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
>> Now that the backtick formats the content as in the synopsis, why is it
>> written _<tree-ish>_ and not `<tree-ish>` in the running text?
> 
> Perhaps that is because the body text does want to show the
> placeholder in _italics_ but not as a `literal` string?

OK, I can agree with that.

-- Hannes

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Jean-Noël Avila wrote (reply to this):

Le 12/11/2024 à 19:51, Johannes Sixt a écrit :
> Am 11.11.24 um 17:53 schrieb Jean-Noël Avila via GitGitGadget:
>> -The raw output format from "git-diff-index", "git-diff-tree",
>> -"git-diff-files" and "git diff --raw" are very similar.
>> +The raw output format from `git-diff-index`, `git-diff-tree`,
>> +`git-diff-files` and `git diff --raw` are very similar.
> 
> Throughout this document we see a lot of commands with dashes `git-foo`.
> Does this have any significance, or should they be corrected to the
> dashless form `git foo`? It could even be a "while at it"-change as you
> are touching every instance anyway.
> 

OK. I didn't pay attention to these points until now.

>>  
>>  These commands all compare two sets of things; what is
>>  compared differs:
>>  
>> -git-diff-index <tree-ish>::
>> -        compares the <tree-ish> and the files on the filesystem.
>> +`git-diff-index <tree-ish>`::
>> +	compares the _<tree-ish>_ and the files on the filesystem.
> 
> Now that the backtick formats the content as in the synopsis, why is it
> written _<tree-ish>_ and not `<tree-ish>` in the running text?
> 

With the new processing in place, this is identical in the result. But
for the writers, I would still push so that the form _<placeholder>_ be
used to remind them that keywords and placeholders need to be
differentiated.

Moreover, in case the special processing macro is not applied, the
markup is still correct pure asciidoc, while generating a "not so bad"
output.

@@ -1,18 +1,18 @@
diff.autoRefreshIndex::
When using 'git diff' to compare with work tree
`diff.autoRefreshIndex`::
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Sixt wrote (reply to this):

Am 11.11.24 um 17:53 schrieb Jean-Noël Avila via GitGitGadget:
> @@ -1,18 +1,18 @@
> -diff.autoRefreshIndex::
> -	When using 'git diff' to compare with work tree
> +`diff.autoRefreshIndex`::
> +	When using `git diff` to compare with work tree
>  	files, do not consider stat-only changes as changed.
>  	Instead, silently run `git update-index --refresh` to
>  	update the cached stat information for paths whose
>  	contents in the work tree match the contents in the
>  	index.  This option defaults to true.  Note that this
> -	affects only 'git diff' Porcelain, and not lower level
> -	'diff' commands such as 'git diff-files'.
> +	affects only `git diff` Porcelain, and not lower level
> +	`diff` commands such as '`git diff-files`.

A stray ' remained on this line.

> -diff.srcPrefix::
> -	If set, 'git diff' uses this source prefix. Defaults to "a/".
> +`diff.srcPrefix`::
> +	If set, `git diff` uses this source prefix. Defaults to "a/".
>  
> -diff.dstPrefix::
> -	If set, 'git diff' uses this destination prefix. Defaults to "b/".
> +`diff.dstPrefix`::
> +	If set, `git diff` uses this destination prefix. Defaults to "b/".

You are applying `backticks` to possible values such as `true` and
`false` later. Then these `a/` and `b/` should also be set in this way.

-- Hannes

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Jean-Noël Avila wrote (reply to this):

Le 12/11/2024 à 19:51, Johannes Sixt a écrit :
> Am 11.11.24 um 17:53 schrieb Jean-Noël Avila via GitGitGadget:
>> @@ -1,18 +1,18 @@
>> -diff.autoRefreshIndex::
>> -	When using 'git diff' to compare with work tree
>> +`diff.autoRefreshIndex`::
>> +	When using `git diff` to compare with work tree
>>  	files, do not consider stat-only changes as changed.
>>  	Instead, silently run `git update-index --refresh` to
>>  	update the cached stat information for paths whose
>>  	contents in the work tree match the contents in the
>>  	index.  This option defaults to true.  Note that this
>> -	affects only 'git diff' Porcelain, and not lower level
>> -	'diff' commands such as 'git diff-files'.
>> +	affects only `git diff` Porcelain, and not lower level
>> +	`diff` commands such as '`git diff-files`.
> 
> A stray ' remained on this line.
> 
>> -diff.srcPrefix::
>> -	If set, 'git diff' uses this source prefix. Defaults to "a/".
>> +`diff.srcPrefix`::
>> +	If set, `git diff` uses this source prefix. Defaults to "a/".
>>  
>> -diff.dstPrefix::
>> -	If set, 'git diff' uses this destination prefix. Defaults to "b/".
>> +`diff.dstPrefix`::
>> +	If set, `git diff` uses this destination prefix. Defaults to "b/".
> 
> You are applying `backticks` to possible values such as `true` and
> `false` later. Then these `a/` and `b/` should also be set in this way.
> 
> -- Hannes
> 

Thank you for your review. Will fix.

Copy link

gitgitgadget bot commented Nov 13, 2024

This patch series was integrated into seen via git@88fcac8.

Copy link

gitgitgadget bot commented Nov 15, 2024

This patch series was integrated into seen via git@ce5ebe9.

Copy link

gitgitgadget bot commented Nov 16, 2024

This patch series was integrated into seen via git@864e672.

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
By the way, we also change the sentences where git-diff would refer to
itself, so that no link is created in the HTML output.

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
@jnavila
Copy link
Author

jnavila commented Nov 18, 2024

/submit

Copy link

gitgitgadget bot commented Nov 18, 2024

Submitted as pull.1769.v4.git.1731967553.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1769/jnavila/git_diff_new-v4

To fetch this version to local tag pr-1769/jnavila/git_diff_new-v4:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1769/jnavila/git_diff_new-v4

Copy link

gitgitgadget bot commented Nov 18, 2024

This patch series was integrated into seen via git@ace108b.

Copy link

gitgitgadget bot commented Nov 19, 2024

This patch series was integrated into seen via git@dbe1a63.

Copy link

gitgitgadget bot commented Nov 20, 2024

This patch series was integrated into seen via git@16cdad2.

Copy link

gitgitgadget bot commented Nov 21, 2024

This patch series was integrated into seen via git@84688c8.

Copy link

gitgitgadget bot commented Nov 22, 2024

This patch series was integrated into seen via git@0275de7.

Copy link

gitgitgadget bot commented Nov 25, 2024

This patch series was integrated into seen via git@7f261e3.

Copy link

gitgitgadget bot commented Nov 25, 2024

This patch series was integrated into seen via git@d076d20.

Copy link

gitgitgadget bot commented Nov 26, 2024

This patch series was integrated into seen via git@27b3c33.

Copy link

gitgitgadget bot commented Nov 26, 2024

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Jean-Noël Avila via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This is the continuation of the editing of the manpages to reflect the new
> formatting rules.
>
> Changes since V3:
>
>  * rework the description about -1, --base,...
>

This latest round did not see any feedback, but I re-read the
changes again, and did not find anything suspicious.  So unless
there are things I missed that others point out quickly, let's
merge this down to 'next'.

Thanks.

Copy link

gitgitgadget bot commented Nov 26, 2024

On the Git mailing list, Johannes Sixt wrote (reply to this):

Am 26.11.24 um 05:32 schrieb Junio C Hamano:
> "Jean-Noël Avila via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> This is the continuation of the editing of the manpages to reflect the new
>> formatting rules.
>>
>> Changes since V3:
>>
>>  * rework the description about -1, --base,...
>>
> 
> This latest round did not see any feedback, but I re-read the
> changes again, and did not find anything suspicious.  So unless
> there are things I missed that others point out quickly, let's
> merge this down to 'next'.

Yes, this round looks good to me, too.

My comment about dashed vs. non-dashed commands in 3/5 hasn't been
addressed, but this is OK since such a change is outside the topic of
this series.

-- Hannes

Copy link

gitgitgadget bot commented Nov 26, 2024

On the Git mailing list, Junio C Hamano wrote (reply to this):

Johannes Sixt <j6t@kdbg.org> writes:

> Yes, this round looks good to me, too.
>
> My comment about dashed vs. non-dashed commands in 3/5 hasn't been
> addressed, but this is OK since such a change is outside the topic of
> this series.

Thanks, let's do that.

Copy link

gitgitgadget bot commented Nov 26, 2024

This patch series was integrated into seen via git@f69a973.

Copy link

gitgitgadget bot commented Nov 26, 2024

This patch series was integrated into next via git@b33b5ae.

@gitgitgadget gitgitgadget bot added the next label Nov 26, 2024
Copy link

gitgitgadget bot commented Nov 28, 2024

This patch series was integrated into seen via git@ba05685.

Copy link

gitgitgadget bot commented Nov 28, 2024

This patch series was integrated into seen via git@6124b16.

Copy link

gitgitgadget bot commented Dec 2, 2024

This patch series was integrated into seen via git@87dbcf3.

Copy link

gitgitgadget bot commented Dec 3, 2024

This patch series was integrated into seen via git@bdd6f7b.

Copy link

gitgitgadget bot commented Dec 4, 2024

This patch series was integrated into seen via git@f334c38.

Copy link

gitgitgadget bot commented Dec 4, 2024

This patch series was integrated into master via git@f334c38.

Copy link

gitgitgadget bot commented Dec 4, 2024

This patch series was integrated into next via git@f334c38.

@gitgitgadget gitgitgadget bot added the master label Dec 4, 2024
Copy link

gitgitgadget bot commented Dec 4, 2024

Closed via f334c38.

@gitgitgadget gitgitgadget bot closed this Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants