| Message ID | 87zg20qzhg.fsf@osv.gnss.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] doc/diff-options: fix link to generating patch section | expand |
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. > 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. > 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[]
Junio C Hamano <gitster@pobox.com> writes: > 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! > > 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
Junio C Hamano <gitster@pobox.com> writes: > 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
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.
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[]
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(-)