doc: git diff reformatting #1769
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jnavila
force-pushed
the
git_diff_new
branch
3 times, most recently
from
4c615e5
to
acb5bdd
Compare
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| @@ -14,7 +14,7 @@ You can customize the creation of patch text via the | |||
| `GIT_EXTERNAL_DIFF` and the `GIT_DIFF_OPTS` environment variables | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Sixt <[email protected]> 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.There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 <[email protected]> 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
There was a problem hiding this comment.
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 <[email protected]> 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.
-- HannesThere was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Sixt <[email protected]> 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 ;-)?There was a problem hiding this comment.
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
|
User |
| @@ -8,13 +8,13 @@ git-diff - Show changes between commits, commit and working tree, etc | |||
|
|
|||
There was a problem hiding this comment.
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?= <[email protected]>
>
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
>
> There was a problem hiding this comment.
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?= <[email protected]>
> >
>
> 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
There was a problem hiding this comment.
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
|
User |
jnavila
force-pushed
the
git_diff_new
branch
3 times, most recently
from
9b47dce
to
17a2f02
Compare
|
/submit |
1 similar comment
|
/submit |
|
Hello @dscho It seems I can no longer submit this PR, the gitgitgadget bot does not react to the submit commentaries. |
|
Hmm. It all started yesterday. |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
@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! |
|
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 |
| @@ -8,13 +8,13 @@ git-diff - Show changes between commits, commit and working tree, etc | |||
|
|
|||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Jean-Noël Avila via GitGitGadget" <[email protected]> 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.There was a problem hiding this comment.
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" <[email protected]> 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.
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Jean-Noël Avila <[email protected]> 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.There was a problem hiding this comment.
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 <[email protected]> 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
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Sixt <[email protected]> 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.There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 <[email protected]> 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[] | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Jean-Noël Avila via GitGitGadget" <[email protected]> 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.There was a problem hiding this comment.
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" <[email protected]> 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.
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Jean-Noël Avila <[email protected]> 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.
|
This patch series was integrated into seen via git@2934ac3. |
| @@ -1,25 +1,25 @@ | |||
| Raw output format | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Sixt <[email protected]> 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?There was a problem hiding this comment.
On the Git mailing list, Johannes Sixt wrote (reply to this):
Am 13.11.24 um 00:03 schrieb Junio C Hamano:
> Johannes Sixt <[email protected]> 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
There was a problem hiding this comment.
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`:: | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
|
This patch series was integrated into seen via git@88fcac8. |
|
This patch series was integrated into seen via git@ce5ebe9. |
jnavila
force-pushed
the
git_diff_new
branch
from
17a2f02
to
4ec2fd9
Compare
|
This patch series was integrated into seen via git@864e672. |
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
| @@ -8,13 +8,13 @@ git-diff - Show changes between commits, commit and working tree, etc | |||
|
|
|||
There was a problem hiding this comment.
On the Git mailing list, Johannes Sixt wrote (reply to this):
Am 16.11.24 um 20:36 schrieb Jean-Noël Avila via GitGitGadget:
> --1 --base::
> --2 --ours::
> --3 --theirs::
> +`-1`::
> +`--base`::
> +
> +or `-2`::
> +`--ours`::
> +
> +or `-3`::
> +`--theirs`::
> Compare the working tree with the "base" version (stage #1),
> "our branch" (stage #2) or "their branch" (stage #3). The
> index contains these stages only for unmerged entries i.e.
> while resolving conflicts. See linkgit:git-read-tree[1]
> section "3-Way Merge" for detailed information.
Having seen this new proposal (which I am not a fan of), I reconsidered
my take on how this could be formatted.
First, I wonder why the pre-image is not
-1::
--base::
-2::
--ours::
-3::
--theirs::
like we write in other cases where multiple options are described by the
same paragraph (e.g.: -p -u --patch; -W --function-context; --textconv
--no-textconv).
Next, since with such a scheme all options are treated equally, we have
to ask whether the description in the body text makes sufficiently clear
that they not all do the same thing (it does), that there are actually 3
distinct groups (it does), and which options mean the same thing. The
latter is rather meh, but it is the fault of the text and can be
remedied easily.
Finally, with all this considered, I think it is not so bad at all that
all options are lumped together in a single line (or remain on six
separate header lines, depending on the processor). So, I would no
longer mind seeing this transformed into
`-1`::
`--base`::
`-2`::
`--ours`::
`-3`::
`--theirs`::
for consistency, or
`-1`, `--base`::
`-2`, `--ours`::
`-3`, `--theirs`::
for brevity.
-- Hannes
There was a problem hiding this comment.
On the Git mailing list, Jean-Noël AVILA wrote (reply to this):
On Sunday, 17 November 2024 15:04:13 CET Johannes Sixt wrote:
> Am 16.11.24 um 20:36 schrieb Jean-Noël Avila via GitGitGadget:
> > --1 --base::
> > --2 --ours::
> > --3 --theirs::
> > +`-1`::
> > +`--base`::
> > +
> > +or `-2`::
> > +`--ours`::
> > +
> > +or `-3`::
> > +`--theirs`::
> > Compare the working tree with the "base" version (stage #1),
> > "our branch" (stage #2) or "their branch" (stage #3). The
> > index contains these stages only for unmerged entries i.e.
> > while resolving conflicts. See linkgit:git-read-tree[1]
> > section "3-Way Merge" for detailed information.
>
> Having seen this new proposal (which I am not a fan of), I reconsidered
> my take on how this could be formatted.
>
> First, I wonder why the pre-image is not
>
> -1::
> --base::
> -2::
> --ours::
> -3::
> --theirs::
>
> like we write in other cases where multiple options are described by the
> same paragraph (e.g.: -p -u --patch; -W --function-context; --textconv
> --no-textconv).
>
> Next, since with such a scheme all options are treated equally, we have
> to ask whether the description in the body text makes sufficiently clear
> that they not all do the same thing (it does), that there are actually 3
> distinct groups (it does), and which options mean the same thing. The
> latter is rather meh, but it is the fault of the text and can be
> remedied easily.
>
OK, I'm not fond of my solution either, but I strongly dislike mixing synonyms
(which is the usual meaning of putting several options in the same
description) with incompatible behavioral alternatives. But, for this one,
let's consider that the alternatives are just like `--[no-]bla` option
descriptions, for the sake of ending this PR.
I would still rephrase the description to make it clear, how the alternatives
are working:
`-1`::
`--base`::
`-2`::
`--ours`::
`-3`::
`--theirs`::
Compare the working tree with
+
--
* the "base" version (stage #1) when using `-1` or `--base`,
* "our branch" (stage #2) when using `-2` or `--ours`, or
* "their branch" (stage #3) when using `-3` or `--theirs`.
--
+
The index contains these stages only for unmerged entries i.e.
while resolving conflicts. See linkgit:git-read-tree[1]
section "3-Way Merge" for detailed information.
> Finally, with all this considered, I think it is not so bad at all that
> all options are lumped together in a single line (or remain on six
> separate header lines, depending on the processor). So, I would no
> longer mind seeing this transformed into
>
> `-1`::
> `--base`::
> `-2`::
> `--ours`::
> `-3`::
> `--theirs`::
>
> for consistency, or
To be honest, this is the form I would prefer because it can be automatically
processed for translation as "do not translate". Any addition involving human
language to a segment requires translation.
Thanks,
JN
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Sixt <[email protected]> writes:
> -1::
> --base::
> -2::
> --ours::
> -3::
> --theirs::
> ...
> Next, since with such a scheme all options are treated equally, we have
> to ask whether the description in the body text makes sufficiently clear
> that they not all do the same thing (it does), that there are actually 3
> distinct groups (it does), and which options mean the same thing. The
> latter is rather meh, but it is the fault of the text and can be
> remedied easily.
OK, with that, making the 6 as the heading at the same level becomes
feasible and the most simple.
> Finally, with all this considered, I think it is not so bad at all that
> all options are lumped together in a single line (or remain on six
> separate header lines, depending on the processor).
Yup. Sounds good.There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Jean-Noël AVILA <[email protected]> writes:
> OK, I'm not fond of my solution either, but I strongly dislike mixing synonyms
> (which is the usual meaning of putting several options in the same
> description) with incompatible behavioral alternatives. But, for this one,
> let's consider that the alternatives are just like `--[no-]bla` option
> descriptions, for the sake of ending this PR.
Makes sense. In this case, not like "--[no-]blah" whose description
has to discuss two options with opposite meaning, we need to
describe three choices.
> I would still rephrase the description to make it clear, how the alternatives
> are working:
Absolutely.
>
> `-1`::
> `--base`::
> `-2`::
> `--ours`::
> `-3`::
> `--theirs`::
> Compare the working tree with
> +
> --
> * the "base" version (stage #1) when using `-1` or `--base`,
> * "our branch" (stage #2) when using `-2` or `--ours`, or
> * "their branch" (stage #3) when using `-3` or `--theirs`.
> --
> +
> The index contains these stages only for unmerged entries i.e.
> while resolving conflicts. See linkgit:git-read-tree[1]
> section "3-Way Merge" for detailed information.
OK.
Thanks.The documentation for git-diff has been updated to follow the new documentation guidelines. The following changes have been applied to the series of patches: - switching the synopsis to a synopsis block which will automatically format placeholders in italics and keywords in monospace - use _<placeholder>_ instead of <placeholder> in the description - use `backticks for keywords and more complex option descriptions`. The new rendering engine will apply synopsis rules to these spans. - prevent git-diff from self-referencing itself via gitlink macro when the generated link would point to the same page. Signed-off-by: Jean-Noël Avila <[email protected]>
The format change is only applied to the sections of the file that are filtered in git-diff. Signed-off-by: Jean-Noël Avila <[email protected]>
Signed-off-by: Jean-Noël Avila <[email protected]>
Signed-off-by: Jean-Noël Avila <[email protected]>
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 <[email protected]>
jnavila
force-pushed
the
git_diff_new
branch
from
4ec2fd9
to
d350237
Compare
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
This patch series was integrated into seen via git@ace108b. |
|
This patch series was integrated into seen via git@dbe1a63. |
|
This patch series was integrated into seen via git@16cdad2. |
|
This patch series was integrated into seen via git@84688c8. |
|
This patch series was integrated into seen via git@0275de7. |
|
This patch series was integrated into seen via git@7f261e3. |
|
This patch series was integrated into seen via git@d076d20. |
|
This patch series was integrated into seen via git@27b3c33. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "Jean-Noël Avila via GitGitGadget" <[email protected]> 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. |
|
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" <[email protected]> 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
|
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Johannes Sixt <[email protected]> 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. |
|
This patch series was integrated into seen via git@f69a973. |
|
This patch series was integrated into next via git@b33b5ae. |
|
This patch series was integrated into seen via git@ba05685. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the continuation of the editing of the manpages to reflect the new formatting rules.
Changes since V3:
-1,--base,...cc: Johannes Sixt [email protected]
cc: Patrick Steinhardt [email protected]