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: convert git-reset, git-rm and git-mv to new documentation format #1896

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jnavila
Copy link

@jnavila jnavila commented Mar 28, 2025

doc: convert git-reset, git-rm and git-mv to new documentation format

change since V1:

  • fix remarks from reviewers
  • stack the commit fixing the handling of three-dot notation

cc: Martin Ågren [email protected]

@jnavila jnavila changed the title doc: convert git-reset to new documentation format doc: convert git-reset, git-rm and git-mv to new documentation format Mar 30, 2025
@jnavila
Copy link
Author

jnavila commented Mar 30, 2025

/submit

Copy link

gitgitgadget bot commented Mar 30, 2025

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1896/jnavila/doc_git_reset-v1

To fetch this version to local tag pr-1896/jnavila/doc_git_reset-v1:

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

Copy link

gitgitgadget bot commented Apr 1, 2025

This patch series was integrated into seen via git@521500a.

@gitgitgadget gitgitgadget bot added the seen label Apr 1, 2025
@@ -7,23 +7,23 @@ git-reset - Reset current HEAD to the specified state

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, Martin Ågren wrote (reply to this):

Hi Jean-Noël,

On Sun, 30 Mar 2025 at 19:16, Jean-Noël Avila via GitGitGadget
<[email protected]> wrote:

> - Switch 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.

> ---soft::
> +`--soft`::
>         Does not touch the index file or the working tree at all (but
> -       resets the head to `<commit>`, just like all modes do). This leaves
> -       all your changed files "Changes to be committed", as `git status`
> +       resets the head to _<commit>_, just like all modes do). This leaves
> +       all your changed files "Changes to be committed", as `git status
>         would put it.

This loses the closing backtick after "git status".

> ---pathspec-from-file=<file>::
> -       Pathspec is passed in `<file>` instead of commandline args. If
> -       `<file>` is exactly `-` then standard input is used. Pathspec
> -       elements are separated by LF or CR/LF. Pathspec elements can be
> +`--pathspec-from-file=<file>`::
> +       Pathspec is passed in _<file>_ instead of commandline args. If
> +       _<file>_ is exactly `-` then standard input is used. Pathspec
> +       elements are separated by _LF_ or _CR_/_LF_. Pathspec elements can be
>         quoted as explained for the configuration variable `core.quotePath`
>         (see linkgit:git-config[1]). See also `--pathspec-file-nul` and
>         global `--literal-pathspecs`.
>
> ---pathspec-file-nul::
> +`--pathspec-file-nul`::
>         Only meaningful with `--pathspec-from-file`. Pathspec elements are
> -       separated with NUL character and all other characters are taken
> +       separated with _NUL_ character and all other characters are taken
>         literally (including newlines and quotes).

I was surprised that you wrapped CR, LF, and NUL in underscores. The
commit message only talks about <placeholders>, similar to
CodingGuidelines. That said, these _CR_ and friends seem to be
consistent with similar conversions you've done before.


Martin

Copy link

gitgitgadget bot commented Apr 4, 2025

User Martin Ågren <[email protected]> has been added to the cc: list.

@@ -43,7 +43,7 @@ ifdef::doctype-book[]
endif::doctype-book[]
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, Martin Ågren wrote (reply to this):

On Sun, 30 Mar 2025 at 19:16, Jean-Noël Avila via GitGitGadget
<[email protected]> wrote:
>
> The synopsis analysis logic was not able to handle backslashes and stars
> which are used in the synopsis of the git-rm command. This patch fixes the
> issue by updating the regular expression used to match the keywords.

> --- a/Documentation/asciidoctor-extensions.rb.in
> +++ b/Documentation/asciidoctor-extensions.rb.in
> @@ -50,7 +50,7 @@ module Git
>        def process parent, reader, attrs
>          outlines = reader.lines.map do |l|
>            l.gsub(/(\.\.\.?)([^\]$.])/, '`\1`\2')
> -           .gsub(%r{([\[\] |()>]|^)([-a-zA-Z0-9:+=~@,/_^\$]+)}, '\1{empty}`\2`{empty}')
> +           .gsub(%r{([\[\] |()>]|^)([-a-zA-Z0-9:+=~@,/_^\$\\\*]+)}, '\1{empty}`\2`{empty}')
>             .gsub(/(<[-a-zA-Z0-9.]+>)/, '__\\1__')
>             .gsub(']', ']{empty}')
>          end
> @@ -72,7 +72,7 @@ module Git
>            %(<inlineequation><alt><![CDATA[#{equation = node.text}]]></alt><mathphrase><![CDATA[#{equation}]]></mathphrase></inlineequation>)
>          elsif type == :monospaced
>            node.text.gsub(/(\.\.\.?)([^\]$.])/, '<literal>\1</literal>\2')
> -              .gsub(%r{([\[\s|()>.]|^|\]|&gt;)(\.?([-a-zA-Z0-9:+=~@,/_^\$]+\.{0,2})+)}, '\1<literal>\2</literal>')
> +              .gsub(%r{([\[\s|()>.]|^|\]|&gt;)(\.?([-a-zA-Z0-9:+=~@,/_^\$\\\*]+\.{0,2})+)}, '\1<literal>\2</literal>')
>                .gsub(/(&lt;[-a-zA-Z0-9.]+&gt;)/, '<emphasis>\1</emphasis>')
>          else
>            open, close, supports_phrase = QUOTE_TAGS[type]
> @@ -100,7 +100,7 @@ module Git
>        def convert_inline_quoted node
>          if node.type == :monospaced
>            node.text.gsub(/(\.\.\.?)([^\]$.])/, '<code>\1</code>\2')
> -              .gsub(%r{([\[\s|()>.]|^|\]|&gt;)(\.?([-a-zA-Z0-9:+=~@,/_^\$]+\.{0,2})+)}, '\1<code>\2</code>')
> +              .gsub(%r{([\[\s|()>.]|^|\]|&gt;)(\.?([-a-zA-Z0-9:+=~@,/_^\$\\\*]+\.{0,2})+)}, '\1<code>\2</code>')
>                .gsub(/(&lt;[-a-zA-Z0-9.]+&gt;)/, '<em>\1</em>')

This seems to introduce some extra spacing in the rendered man pages, e.g.,
"The bundle.*  keys" or "Fileglobs (e.g.  *.c)". (Asciidoctor 2.0.18.) I
haven't dug into the regexes so see what might be the cause.

(I only had time to have a look at the first patch, then briefly trying out
this one.)


Martin

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 05/04/2025 à 00:39, Martin Ågren a écrit :
> On Sun, 30 Mar 2025 at 19:16, Jean-Noël Avila via GitGitGadget
> <[email protected]> wrote:
>>
>> The synopsis analysis logic was not able to handle backslashes and stars
>> which are used in the synopsis of the git-rm command. This patch fixes the
>> issue by updating the regular expression used to match the keywords.
> 
>> --- a/Documentation/asciidoctor-extensions.rb.in
>> +++ b/Documentation/asciidoctor-extensions.rb.in
>> @@ -50,7 +50,7 @@ module Git
>>        def process parent, reader, attrs
>>          outlines = reader.lines.map do |l|
>>            l.gsub(/(\.\.\.?)([^\]$.])/, '`\1`\2')
>> -           .gsub(%r{([\[\] |()>]|^)([-a-zA-Z0-9:+=~@,/_^\$]+)}, '\1{empty}`\2`{empty}')
>> +           .gsub(%r{([\[\] |()>]|^)([-a-zA-Z0-9:+=~@,/_^\$\\\*]+)}, '\1{empty}`\2`{empty}')
>>             .gsub(/(<[-a-zA-Z0-9.]+>)/, '__\\1__')
>>             .gsub(']', ']{empty}')
>>          end
>> @@ -72,7 +72,7 @@ module Git
>>            %(<inlineequation><alt><![CDATA[#{equation = node.text}]]></alt><mathphrase><![CDATA[#{equation}]]></mathphrase></inlineequation>)
>>          elsif type == :monospaced
>>            node.text.gsub(/(\.\.\.?)([^\]$.])/, '<literal>\1</literal>\2')
>> -              .gsub(%r{([\[\s|()>.]|^|\]|&gt;)(\.?([-a-zA-Z0-9:+=~@,/_^\$]+\.{0,2})+)}, '\1<literal>\2</literal>')
>> +              .gsub(%r{([\[\s|()>.]|^|\]|&gt;)(\.?([-a-zA-Z0-9:+=~@,/_^\$\\\*]+\.{0,2})+)}, '\1<literal>\2</literal>')
>>                .gsub(/(&lt;[-a-zA-Z0-9.]+&gt;)/, '<emphasis>\1</emphasis>')
>>          else
>>            open, close, supports_phrase = QUOTE_TAGS[type]
>> @@ -100,7 +100,7 @@ module Git
>>        def convert_inline_quoted node
>>          if node.type == :monospaced
>>            node.text.gsub(/(\.\.\.?)([^\]$.])/, '<code>\1</code>\2')
>> -              .gsub(%r{([\[\s|()>.]|^|\]|&gt;)(\.?([-a-zA-Z0-9:+=~@,/_^\$]+\.{0,2})+)}, '\1<code>\2</code>')
>> +              .gsub(%r{([\[\s|()>.]|^|\]|&gt;)(\.?([-a-zA-Z0-9:+=~@,/_^\$\\\*]+\.{0,2})+)}, '\1<code>\2</code>')
>>                .gsub(/(&lt;[-a-zA-Z0-9.]+&gt;)/, '<em>\1</em>')
> 
> This seems to introduce some extra spacing in the rendered man pages, e.g.,
> "The bundle.*  keys" or "Fileglobs (e.g.  *.c)". (Asciidoctor 2.0.18.) I
> haven't dug into the regexes so see what might be the cause.
> 

The xml regex seems ok for this. The docbook output is as follows:

(...)
pairs in this list are in the <literal>bundle.*</literal> namespace (see
(...)

The manpage output seems also correct:

(...)
would accept (with the
\fB\-\-file\fR
option)\&. The key\-value pairs in this list are in the
\fBbundle\&.*\fR
namespace (see
(...)

Strangely, the --file above is rendered correctly, but the bundle.*
below not. I do not know TROFF to assess what is going on. Has '*' a
special behavior in a bold span?

JN

@@ -9,15 +9,13 @@ git-mv - Move or rename a file, a directory, or a symlink
SYNOPSIS
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, Martin Ågren wrote (reply to this):

On Sun, 30 Mar 2025 at 19:16, Jean-Noël Avila via GitGitGadget
<[email protected]> wrote:

>  [verse]
> -'git mv' [<options>] <source>... <destination>
> +'git mv' [-v] [-f] [-n] [-k] <source> <destination>
> +'git mv' [-v] [-f] [-n] [-k] <source> ... <destination-directory>

Shouldn't "..." be tucked with the preceding "<source>", i.e.,
"<source>..." as it was in the original?

>  DESCRIPTION
>  -----------
>  Move or rename a file, directory, or symlink.
>
> - git mv [-v] [-f] [-n] [-k] <source> <destination>
> - git mv [-v] [-f] [-n] [-k] <source> ... <destination-directory>
> -

This is where "<source> ..." comes from. You moved these lines up. Good.
But I do think we want "<source>..."?

Martin

Copy link

gitgitgadget bot commented Apr 7, 2025

This branch is now known as ja/doc-reset-mv-rm-markup-updates.

Copy link

gitgitgadget bot commented Apr 7, 2025

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

Copy link

gitgitgadget bot commented Apr 8, 2025

This patch series was integrated into seen via git@79ed5a8.

Copy link

gitgitgadget bot commented Apr 8, 2025

This patch series was integrated into seen via git@0d747b8.

Copy link

gitgitgadget bot commented Apr 11, 2025

This patch series was integrated into seen via git@30174dc.

Copy link

gitgitgadget bot commented Apr 11, 2025

There was a status update in the "Cooking" section about the branch ja/doc-reset-mv-rm-markup-updates on the Git mailing list:

Expecting a reroll.
cf. <CAN0heSodC8_Uwg_Lw31rtkdLfOEDyGg=iE0gb1TRrUWQEynT+w@mail.gmail.com>
cf. <[email protected]>
source: <[email protected]>

jnavila added 6 commits April 12, 2025 17:55
- Switch 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.

Signed-off-by: Jean-Noël Avila <[email protected]>
The synopsis analysis logic was not able to handle backslashes and stars
which are used in the synopsis of the git-rm command. This patch fixes the
issue by updating the regular expression used to match the keywords.

Signed-off-by: Jean-Noël Avila <[email protected]>
- Switch 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.

Signed-off-by: Jean-Noël Avila <[email protected]>
This also entails changing the help output for the command to match the new
synopsis.

Signed-off-by: Jean-Noël Avila <[email protected]>
- Switch 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.

Unfortunately, there's an inconsistency in the synopsis style, where
the ellipsis is used to indicate that the option can be repeated, but
it can also be used in Git's three-dot notation to indicate a range of
commits. The rendering engine will not be able to distinguish
between these two cases.

Signed-off-by: Jean-Noël Avila <[email protected]>
The processing of triple dot notation is tricky because it can be
mis-interpreted as an ellipsis. The special processing of the ellipsis
is now complete and takes into account the case of
`git-mv <source>... <dest>`

Signed-off-by: Jean-Noël Avila <[email protected]>
This rule was already implicitely applied in the converted man pages,
so let's state it loudly.

Signed-off-by: Jean-Noël Avila <[email protected]>
@jnavila
Copy link
Author

jnavila commented Apr 12, 2025

/submit

Copy link

gitgitgadget bot commented Apr 12, 2025

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1896/jnavila/doc_git_reset-v2

To fetch this version to local tag pr-1896/jnavila/doc_git_reset-v2:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant