[1/6] doc: log, gitk: move '-L' description to 'line-range-options.txt'

Commit Message

Philippe Blain Oct. 28, 2020, 12:47 p.m. UTC

From: Philippe Blain <[e-mail]>

The description of the '-L' option for `git log` and `gitk` is the
same, but is repeated in both 'git-log.txt' and 'gitk.txt'.

Remove the duplication by creating a new file, 'line-range-options.txt',
and include it in both files.

Signed-off-by: Philippe Blain <[e-mail]>
---
 Documentation/git-log.txt            | 16 ++--------------
 Documentation/gitk.txt               | 21 ++-------------------
 Documentation/line-range-options.txt | 27 +++++++++++++++++++++++++++
 3 files changed, 31 insertions(+), 33 deletions(-)
 create mode 100644 Documentation/line-range-options.txt

Comments

Junio C Hamano Oct. 29, 2020, 8:16 p.m. UTC | #1

"Philippe Blain via GitGitGadget" <[e-mail]> writes:

> From: Philippe Blain <[e-mail]>
>
> The description of the '-L' option for `git log` and `gitk` is the
> same, but is repeated in both 'git-log.txt' and 'gitk.txt'.

They are moral equivalents but differ slightly, both in
an insignificant way (i.e. SP after -L and its parameters) and a
significant way (i.e. Note about gitk usage), so "the same, but is
repeated" is not sufficient to explain why the contents of the new
file looks the way it is.

> Remove the duplication by creating a new file, 'line-range-options.txt',
> and include it in both files.

Makes sense.  As to the conditional, I actually think the version
with SP after -L do not have to be listed and instead to show just
the "stuck" form as the standardised way.  If the option takes an
optional value, you must have to use the "stuck" form anyway, and
showing that you _could_ have SP there unnecessarily throws extra
bytes at the reader with no real gain.

> diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
> index c653ebb6a8..761b764c18 100644
> --- a/Documentation/gitk.txt
> +++ b/Documentation/gitk.txt
> @@ -98,25 +98,8 @@ linkgit:git-rev-list[1] for a complete list.
>  	(See "History simplification" in linkgit:git-log[1] for a more
>  	detailed explanation.)
>  
> --L<start>,<end>:<file>::
> --L:<funcname>:<file>::
> ...
> -	`--name-only`, `--name-status`, `--check`) are not currently implemented.
> -+
> -*Note:* gitk (unlike linkgit:git-log[1]) currently only understands
> -this option if you specify it "glued together" with its argument.  Do
> -*not* put a space after `-L`.
> -+
> -include::line-range-format.txt[]
> +:gitk: 1
> +include::line-range-options.txt[]

It looked puzzling to see the inclusion of line-range-format is lost
from here, but it turns out to be OK after all when we look at the
new line-range-options file.

> diff --git a/Documentation/line-range-options.txt b/Documentation/line-range-options.txt
> new file mode 100644
> index 0000000000..9e3d98d44f
> --- /dev/null
> +++ b/Documentation/line-range-options.txt
> @@ -0,0 +1,27 @@
> +ifdef::git-log[]
> +-L <start>,<end>:<file>::
> +-L :<funcname>:<file>::
> +endif::git-log[]
> +ifdef::gitk[]
> +-L<start>,<end>:<file>::
> +-L:<funcname>:<file>::
> +endif::gitk[]

As I said, my vote goes to reducing the variations and using only
the latter form for simplicity.

> +	Trace the evolution of the line range given by "<start>,<end>"
> +	(or the function name regex <funcname>) within the <file>.  You may
> +	not give any pathspec limiters.  This is currently limited to
> +	a walk starting from a single revision, i.e., you may only
> +	give zero or one positive revision arguments, and
> +	<start> and <end> (or <funcname>) must exist in the starting revision.
> +	You can specify this option more than once. Implies `--patch`.
> +	Patch output can be suppressed using `--no-patch`, but other diff formats
> +	(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,
> +	`--name-only`, `--name-status`, `--check`) are not currently implemented.

This text is the same as both originals, which is good.

> ++
> +ifdef::gitk[]
> +*Note:* gitk (unlike linkgit:git-log[1]) currently only understands
> +this option if you specify it "glued together" with its argument.  Do
> +*not* put a space after `-L`.
> +endif::gitk[]

I think it may make sense to keep this part, mostly from "keep the
result as close to the original as possible while refactoring"
principle, but if we only teach the "stuck" form in "git log", it
becomes irrelevant---one fewer thing the users need to learn and
remember, it becomes easier for them to concentrate on what really
matters.


> +include::line-range-format.txt[]

And the answer to my earlier puzzlement is here.  I actually would
have expected that it would be the matter of moving the common text
you moved from gitk/log documentation to this file instead to the
top of the line-range-format.txt file, but the latter is included
also by blame-options.txt, and the description of the option needs
to be somewhat different between blame and log anyway, so I think
this is the best we can do at this step in the series.

Looking good so far...

Philippe Blain Oct. 31, 2020, 5:18 p.m. UTC | #2

Hi Junio,

> Le 29 oct. 2020 à 16:16, Junio C Hamano <[e-mail]> a écrit :
> 
> "Philippe Blain via GitGitGadget" <[e-mail]> writes:
> 
>> From: Philippe Blain <[e-mail]>
>> 
>> The description of the '-L' option for `git log` and `gitk` is the
>> same, but is repeated in both 'git-log.txt' and 'gitk.txt'.
> 
> They are moral equivalents but differ slightly, both in
> an insignificant way (i.e. SP after -L and its parameters) and a
> significant way (i.e. Note about gitk usage), so "the same, but is
> repeated" is not sufficient to explain why the contents of the new
> file looks the way it is.

OK. I'll expand on that.

>> Remove the duplication by creating a new file, 'line-range-options.txt',
>> and include it in both files.
> 
> Makes sense.  As to the conditional, I actually think the version
> with SP after -L do not have to be listed and instead to show just
> the "stuck" form as the standardised way.  If the option takes an
> optional value, you must have to use the "stuck" form anyway, and
> showing that you _could_ have SP there unnecessarily throws extra
> bytes at the reader with no real gain.

I'm not sure I understand here... The <range> argument is not optional
here (`git log -L` by itself is meaningless)... I know gitcli(7) recommends the
stuck form, but this option has been listed as `-L <range>` in the git-log
documentation ever since it appeared... So I did not think it wise to change it, 
that's why I introduce the conditional.

Thanks,

Philippe.

Junio C Hamano Oct. 31, 2020, 5:29 p.m. UTC | #3

Philippe Blain <[e-mail]> writes:

>> Makes sense.  As to the conditional, I actually think the version
>> with SP after -L do not have to be listed and instead to show just
>> the "stuck" form as the standardised way.  If the option takes an
>> optional value, you must have to use the "stuck" form anyway, and
>> showing that you _could_ have SP there unnecessarily throws extra
>> bytes at the reader with no real gain.
>
> I'm not sure I understand here... The <range> argument is not optional
> here (`git log -L` by itself is meaningless)... I know gitcli(7) recommends the
> stuck form, ...

I do not think showing alternative ways distracts without adding
much value.  We should remember that most users are learning Git to
only to do things they want to do and not necessarily learning Git
to master all the slightly different ways to do the same thing.
"You can write option and its argument separately most of the time
but you must write option and its argument in the stuck form some
other times---you must think if the argument is optional or not,
whenyou want to write your argument separately" crams more things to
learn than the simple "Write option and its arguments in the stuck
form and it always works".

Patch

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 2b8ac5ff88..631dfff77c 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -77,20 +77,8 @@  produced by `--stat`, etc.
 	Intended to speed up tools that read log messages from `git log`
 	output by allowing them to allocate space in advance.
 
--L <start>,<end>:<file>::
--L :<funcname>:<file>::
-	Trace the evolution of the line range given by "<start>,<end>"
-	(or the function name regex <funcname>) within the <file>.  You may
-	not give any pathspec limiters.  This is currently limited to
-	a walk starting from a single revision, i.e., you may only
-	give zero or one positive revision arguments, and
-	<start> and <end> (or <funcname>) must exist in the starting revision.
-	You can specify this option more than once. Implies `--patch`.
-	Patch output can be suppressed using `--no-patch`, but other diff formats
-	(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,
-	`--name-only`, `--name-status`, `--check`) are not currently implemented.
-+
-include::line-range-format.txt[]
+:git-log: 1
+include::line-range-options.txt[]
 
 <revision range>::
 	Show only commits in the specified revision range.  When no
diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index c653ebb6a8..761b764c18 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -98,25 +98,8 @@  linkgit:git-rev-list[1] for a complete list.
 	(See "History simplification" in linkgit:git-log[1] for a more
 	detailed explanation.)
 
--L<start>,<end>:<file>::
--L:<funcname>:<file>::
-
-	Trace the evolution of the line range given by "<start>,<end>"
-	(or the function name regex <funcname>) within the <file>.  You may
-	not give any pathspec limiters.  This is currently limited to
-	a walk starting from a single revision, i.e., you may only
-	give zero or one positive revision arguments, and
-	<start> and <end> (or <funcname>) must exist in the starting revision.
-	You can specify this option more than once. Implies `--patch`.
-	Patch output can be suppressed using `--no-patch`, but other diff formats
-	(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,
-	`--name-only`, `--name-status`, `--check`) are not currently implemented.
-+
-*Note:* gitk (unlike linkgit:git-log[1]) currently only understands
-this option if you specify it "glued together" with its argument.  Do
-*not* put a space after `-L`.
-+
-include::line-range-format.txt[]
+:gitk: 1
+include::line-range-options.txt[]
 
 <revision range>::
 
diff --git a/Documentation/line-range-options.txt b/Documentation/line-range-options.txt
new file mode 100644
index 0000000000..9e3d98d44f
--- /dev/null
+++ b/Documentation/line-range-options.txt
@@ -0,0 +1,27 @@ 
+ifdef::git-log[]
+-L <start>,<end>:<file>::
+-L :<funcname>:<file>::
+endif::git-log[]
+ifdef::gitk[]
+-L<start>,<end>:<file>::
+-L:<funcname>:<file>::
+endif::gitk[]
+
+	Trace the evolution of the line range given by "<start>,<end>"
+	(or the function name regex <funcname>) within the <file>.  You may
+	not give any pathspec limiters.  This is currently limited to
+	a walk starting from a single revision, i.e., you may only
+	give zero or one positive revision arguments, and
+	<start> and <end> (or <funcname>) must exist in the starting revision.
+	You can specify this option more than once. Implies `--patch`.
+	Patch output can be suppressed using `--no-patch`, but other diff formats
+	(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,
+	`--name-only`, `--name-status`, `--check`) are not currently implemented.
++
+ifdef::gitk[]
+*Note:* gitk (unlike linkgit:git-log[1]) currently only understands
+this option if you specify it "glued together" with its argument.  Do
+*not* put a space after `-L`.
+endif::gitk[]
++
+include::line-range-format.txt[]