threads / patch / 60200

patchdoc/diff-options: fix link to generating patch section

Subject: [PATCH 1/1] doc/diff-options: fix link to generating patch section

## tl;dr

6 messages between Sep 5, 2023 and Sep 6, 2023. Diffs are folded; open one to read it.

replies: 5people: 2as markdown or json

Sergey Organov· Sep 5, 2023, 17:52 UTC · lore

First, there is no need for conditional referencing, as all the files that include "diff-options.txt" eventually include "diff-generate-patch.txt" as well.

Next, when formatted as man-page, the section title is rendered "GENERATING PATCH TEXT WITH -P" whereas reference still reads "Generating patch text with -p", that is both inconsistent and makes searching harder than it needs to be.

Fix the issues by just referring to the section, without custom reference text, and then unconditionally.

Fixes: ebdc46c242 (docs: link generating patch sections)
Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
Show changes to Documentation/diff-options.txt +1 −7
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9f33f887711d..c07488b123c6 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -22,13 +22,7 @@ ifndef::git-format-patch[]
 -p::
 -u::
 --patch::
-	Generate patch (see section titled
-ifdef::git-log[]
-<<generate_patch_text_with_p, "Generating patch text with -p">>).
-endif::git-log[]
-ifndef::git-log[]
-"Generating patch text with -p").
-endif::git-log[]
+	Generate patch (see <<generate_patch_text_with_p>>).
 ifdef::git-diff[]
 	This is the default.
 endif::git-diff[]
-- 
2.25.1
Junio C Hamano· Sep 5, 2023, 22:49 UTC · re: Sergey Organov · lore

Re: [PATCH 1/1] doc/diff-options: fix link to generating patch section

Sergey Organov <sorganov@gmail.com> writes:
> First, there is no need for conditional referencing, as all the files
> that include "diff-options.txt" eventually include
> "diff-generate-patch.txt" as well.

Except for git-format-patch.txt which includes the former but not the latter. But this is inside ifndef::git-format-patch[], so the above description being a bit imprecise does not cause any actual damage.

    Documentation for all commands that want to describe the `-p`
    option by including the "diff-options.txt" file also include the
    "diff-generate-patch.txt" file, so an internal link would work
    for all of them.
or something like that, perhaps.
Show 7 quoted lines
> Next, when formatted as man-page, the section title is rendered
> "GENERATING PATCH TEXT WITH -P" whereas reference still reads
> "Generating patch text with -p", that is both inconsistent and makes
> searching harder than it needs to be.
>
> Fix the issues by just referring to the section, without custom
> reference text, and then unconditionally.
That does make sense.
Show 25 quoted lines
> Fixes: ebdc46c242 (docs: link generating patch sections)
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
>  Documentation/diff-options.txt | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 9f33f887711d..c07488b123c6 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -22,13 +22,7 @@ ifndef::git-format-patch[]
>  -p::
>  -u::
>  --patch::
> -	Generate patch (see section titled
> -ifdef::git-log[]
> -<<generate_patch_text_with_p, "Generating patch text with -p">>).
> -endif::git-log[]
> -ifndef::git-log[]
> -"Generating patch text with -p").
> -endif::git-log[]
> +	Generate patch (see <<generate_patch_text_with_p>>).
>  ifdef::git-diff[]
>  	This is the default.
>  endif::git-diff[]
Sergey Organov· Sep 6, 2023, 06:40 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/1] doc/diff-options: fix link to generating patch section

Junio C Hamano <gitster@pobox.com> writes:
Show 10 quoted lines
> Sergey Organov <sorganov@gmail.com> writes:
>
>> First, there is no need for conditional referencing, as all the files
>> that include "diff-options.txt" eventually include
>> "diff-generate-patch.txt" as well.
>
> Except for git-format-patch.txt which includes the former but not
> the latter.  But this is inside ifndef::git-format-patch[], so the
> above description being a bit imprecise does not cause any actual
> damage.
Ah, nice catch!
Show 5 quoted lines
>
>     Documentation for all commands that want to describe the `-p`
>     option by including the "diff-options.txt" file also include the
>     "diff-generate-patch.txt" file, so an internal link would work
>     for all of them.

Sounds fine. Would you re-phrase it yourself, or should I rather re-roll?

Thanks, -- Sergey Organov

Sergey Organov· Sep 6, 2023, 06:47 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/1] doc/diff-options: fix link to generating patch section

Junio C Hamano <gitster@pobox.com> writes:
Show 17 quoted lines
> Sergey Organov <sorganov@gmail.com> writes:
>
>> First, there is no need for conditional referencing, as all the files
>> that include "diff-options.txt" eventually include
>> "diff-generate-patch.txt" as well.
>
> Except for git-format-patch.txt which includes the former but not
> the latter.  But this is inside ifndef::git-format-patch[], so the
> above description being a bit imprecise does not cause any actual
> damage.
>
>     Documentation for all commands that want to describe the `-p`
>     option by including the "diff-options.txt" file also include the
>     "diff-generate-patch.txt" file, so an internal link would work
>     for all of them.
>
> or something like that, perhaps.

In fact I just realized that removing conditionals in fact *fixes* those documents by providing proper link in them as well, so I'll think of better description taking into account your observation as well, and then will re-roll.

Thanks, -- Sergey Organov

Junio C Hamano· Sep 6, 2023, 15:58 UTC · re: Sergey Organov · lore

Re: [PATCH 1/1] doc/diff-options: fix link to generating patch section

Sergey Organov <sorganov@gmail.com> writes:
> In fact I just realized that removing conditionals in fact *fixes* those
> documents by providing proper link in them as well, so I'll think of
> better description taking into account your observation as well, and
> then will re-roll.
Thanks.
Sergey Organov· Sep 6, 2023, 07:15 UTC · re: Junio C Hamano · lore

[PATCH v2 1/1] doc/diff-options: fix link to generating patch section

When formatted as man-page, the section title is rendered "GENERATING PATCH TEXT WITH -P" whereas reference still reads "Generating patch text with -p", that is inconsistent and makes searching harder than it needs to be.

Fix this by getting rid of custom reference text.

Also, documentation for every command that describes `-p` option by including the "diff-options.txt" file does include the "diff-generate-patch.txt" file as well (as it should), so the internal link is in fact useful for any of them.

Fix this by getting rid of conditionals around the reference.
Fixes: ebdc46c242 (docs: link generating patch sections)
Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
Show changes to Documentation/diff-options.txt +1 −7
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9f33f887711d..c07488b123c6 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -22,13 +22,7 @@ ifndef::git-format-patch[]
 -p::
 -u::
 --patch::
-	Generate patch (see section titled
-ifdef::git-log[]
-<<generate_patch_text_with_p, "Generating patch text with -p">>).
-endif::git-log[]
-ifndef::git-log[]
-"Generating patch text with -p").
-endif::git-log[]
+	Generate patch (see <<generate_patch_text_with_p>>).
 ifdef::git-diff[]
 	This is the default.
 endif::git-diff[]
-- 
2.25.1

← back to recent threads