git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, "Thomas Rast" <tr@thomasrast.ch>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"René Scharfe" <l.s.r@web.de>,
	"Philippe Blain" <levraiphilippeblain@gmail.com>
Subject: Re: [PATCH 1/6] doc: log, gitk: move '-L' description to 'line-range-options.txt'
Date: Thu, 29 Oct 2020 13:16:55 -0700	[thread overview]
Message-ID: <xmqqft5w7poo.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <96f6f95abcbd79d432073cb294ba12b71300580f.1603889270.git.gitgitgadget@gmail.com> (Philippe Blain via GitGitGadget's message of "Wed, 28 Oct 2020 12:47:45 +0000")

"Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes:

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

  reply	other threads:[~2020-10-29 20:17 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-28 12:47 [PATCH 0/6] blame: enable funcname blaming with userdiff driver Philippe Blain via GitGitGadget
2020-10-28 12:47 ` [PATCH 1/6] doc: log, gitk: move '-L' description to 'line-range-options.txt' Philippe Blain via GitGitGadget
2020-10-29 20:16   ` Junio C Hamano [this message]
2020-10-31 17:18     ` Philippe Blain
2020-10-31 17:29       ` Junio C Hamano
2020-10-28 12:47 ` [PATCH 2/6] doc: line-range: improve formatting Philippe Blain via GitGitGadget
2020-10-28 17:23   ` Eric Sunshine
2020-10-29 20:19     ` Junio C Hamano
2020-10-31 17:20       ` Philippe Blain
2020-10-28 12:47 ` [PATCH 3/6] blame-options.txt: also mention 'funcname' in '-L' description Philippe Blain via GitGitGadget
2020-10-29 20:25   ` Junio C Hamano
2020-10-31 17:22     ` Philippe Blain
2020-10-28 12:47 ` [PATCH 4/6] doc: add more pointers to gitattributes(5) for userdiff Philippe Blain via GitGitGadget
2020-10-28 17:26   ` Eric Sunshine
2020-10-28 12:47 ` [PATCH 5/6] line-log: mention both modes in 'blame' and 'log' short help Philippe Blain via GitGitGadget
2020-10-28 17:33   ` Eric Sunshine
2020-10-29 12:43     ` Philippe Blain
2020-10-28 12:47 ` [PATCH 6/6] blame: enable funcname blaming with userdiff driver Philippe Blain via GitGitGadget
2020-10-29 20:40   ` Junio C Hamano
2020-10-31 18:02     ` Philippe Blain
2020-10-31 18:58       ` Junio C Hamano
2020-11-01 17:28 ` [PATCH v2 0/8] " Philippe Blain via GitGitGadget
2020-11-01 17:28   ` [PATCH v2 1/8] doc: log, gitk: move '-L' description to 'line-range-options.txt' Philippe Blain via GitGitGadget
2020-11-01 17:28   ` [PATCH v2 2/8] doc: line-range: improve formatting Philippe Blain via GitGitGadget
2020-11-01 17:28   ` [PATCH v2 3/8] blame-options.txt: also mention 'funcname' in '-L' description Philippe Blain via GitGitGadget
2020-11-01 17:28   ` [PATCH v2 4/8] doc: add more pointers to gitattributes(5) for userdiff Philippe Blain via GitGitGadget
2020-11-01 17:28   ` [PATCH v2 5/8] line-log: mention both modes in 'blame' and 'log' short help Philippe Blain via GitGitGadget
2020-11-01 17:28   ` [PATCH v2 6/8] blame: enable funcname blaming with userdiff driver Philippe Blain via GitGitGadget
2020-11-01 17:28   ` [PATCH v2 7/8] blame: simplify 'setup_scoreboard' interface Philippe Blain via GitGitGadget
2020-11-01 17:28   ` [PATCH v2 8/8] blame: simplify 'setup_blame_bloom_data' interface Philippe Blain via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqft5w7poo.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=l.s.r@web.de \
    --cc=levraiphilippeblain@gmail.com \
    --cc=sunshine@sunshineco.com \
    --cc=tr@thomasrast.ch \
    --subject='Re: [PATCH 1/6] doc: log, gitk: move '\''-L'\'' description to '\''line-range-options.txt'\''' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).