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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Philippe Blain via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:

> From: Philippe Blain <levraiphilippeblain@xxxxxxxxx>
>
> 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...



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux