Re: [PATCH v4 1/1] cat-file: add mailmap subcommand to --batch-command

[Junio C Hamano <gitster@pobox.com>]

Siddharth Asthana <siddharthasthana31@gmail.com> writes:

> diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc
> index c139f55a16..0f499c9d1b 100644
> --- a/Documentation/git-cat-file.adoc
> +++ b/Documentation/git-cat-file.adoc
> @@ -174,6 +174,18 @@ flush::
>  	since the beginning or since the last flush was issued. When `--buffer`
>  	is used, no output will come until a `flush` is issued. When `--buffer`
>  	is not used, commands are flushed each time without issuing `flush`.
> +
> +`mailmap (<bool>)`::
> +	Enable or disable mailmap for subsequent commands. The `<bool>`
> +	argument accepts the same boolean values as linkgit:git-config[1].
> +	Possible effects are:
> ++
> +`true`;;
> +	Mailmap data is loaded on first use and kept in memory until the
> +	process exits. Passing `true` again does not reload the data.
> +`false`;;
> +	Mailmap replacements are disabled for subsequent commands, but data
> +	already loaded stays in memory.
>  --
>  +

While the above may not be telling any lies, I think the focus of
the explanation is placed on a wrong thing.  What primarily matters
to end-users is that toggling 'mailmap' to true makes the mailmap
applied to identities before they appear in the output and false
makes the identities output without modification.  The fact that you
read the mailmap data only once and keep it around even when
toggling the feature off, just in case the feature gets turned on
again, is an implementation detail that is of much lessor interest
to end users, no?

Perhaps delete everything from "Possible effects are" and replace it
with a brief explanation, e.g.,

`mailmap (<bool>)`::
	Enable or disable mailmap for subsequent commands. The `<bool>`
	argument accepts the same boolean values as linkgit:git-config[1].
	The mailmap data is read upon the first use and only once,
	even after the `mailmap` command is given multiple times to
	toggle it off and then on back again.

would be sufficient (I would omit "even after ..." part, if I were
writing it).  What is more important than the optimization aspect of
this implementation detail to end users is that we do not re-read,
so if you update the file while you are running "cat-file --batch",
even giving 'mailmap off; mailmap on' would not cause it to re-read
the updated data, and "upon the first use and only once" should be
sufficient to understand that.

The implementation looks great.  I do not recall how good the tests
were but this round does not change anything there since the
previous iteration.