Re: [PATCH] doc/cat-file: allow --use-mailmap for --batch options

> On Sat, Oct 29, 2022 at 11:25 AM Siddharth Asthana
> <siddharthasthana31@gmail.com> wrote:
>>
>> The command git cat-file can now use the mailmap mechanism to replace
>> idents with their canonical versions for commit and tag objects. There
>> are several options like `--batch`, `--batch-check` and
>> `--batch-command` that can be combined with `--use-mailmap`. But, the
>> documentation for `--batch`, `--batch-check` and `--batch-command`
>> doesn't say so. This patch fixes that documentation.
>>
>> Mentored-by: Christian Couder <christian.couder@gmail.com>
>> Mentored-by: John Cai <johncai86@gmail.com>
>> Signed-off-by: Siddharth Asthana <siddharthasthana31@gmail.com>
>> ---
>>
>> This patch was previously sent as the first patch of a 3 patch series for
>> adding the mailmap support in git cat-file options:
>> https://lore.kernel.org/git/20220916205946.178925-2-siddharthasthana31@gmail.com/
> 
> I think Taylor is probably right that there should be a version number
> at the start of the subject line if the patch has already been sent
> many times with the same subject and mostly the same content to the
> list.
> 
>> Changes in v2:
>> - Fixed the names in trailers in the commit message
>> - Updated the documentation to clearly state that the `--batch-check`,
>>    `--batch-command` and `--batch` options can only be used with
>>    `--textconv`, `--filters` or `--use-mailmap`.
>> - Fixed formatting
>>
>> Changes in v3:
>> - Fixed formatting
>>
>> Changes in v4:
>> - Fixed some grammatical errors
> 
> The version number should logically be "v4" according to the above.
> 
>>   Documentation/git-cat-file.txt | 45 ++++++++++++++++++++++++----------
>>   1 file changed, 32 insertions(+), 13 deletions(-)
>>
>> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
>> index ec30b5c574..db2bba9b65 100644
>> --- a/Documentation/git-cat-file.txt
>> +++ b/Documentation/git-cat-file.txt
>> @@ -89,26 +89,45 @@ OPTIONS
>>   --batch::
>>   --batch=<format>::
>>          Print object information and contents for each object provided
>> -       on stdin.  May not be combined with any other options or arguments
>> -       except `--textconv` or `--filters`, in which case the input lines
>> -       also need to specify the path, separated by whitespace.  See the
>> -       section `BATCH OUTPUT` below for details.
>> +       on stdin. May not be combined with any other options or arguments
>> +       except --textconv, --filters, or --use-mailmap.
>> +       +
>> +       * When used with `--textconv` or `--filters`, the input lines
>> +         must specify the path, separated by whitespace. See the section
>> +         `BATCH OUTPUT` below for details.
>> +       +
>> +       * When used with `--use-mailmap`, the info command shows the size
>> +         of the object as if the identities recorded in it were replaced
>> +         by the mailmap mechanism.
> 
> Sorry to realize this only at this point but actually I think the
> `info` command is only available when the `--batch-command` option is
> used, not when the `--batch` option is used.
> 
> As the `--batch` option prints both object information (which includes
> the size) and contents (which includes the idents), I think something
> like the following should be used here:
> 
>         * When used with `--use-mailmap`, for commit and tag objects, the
>            contents part of the output shows the identities replaced using the
>            mailmap mechanism, while the information part of the output shows
>            the size of the object as if it actually recorded the replacement
>            identities.
> 
>>   --batch-check::
>>   --batch-check=<format>::
>> -       Print object information for each object provided on stdin.  May
>> -       not be combined with any other options or arguments except
>> -       `--textconv` or `--filters`, in which case the input lines also
>> -       need to specify the path, separated by whitespace.  See the
>> -       section `BATCH OUTPUT` below for details.
>> +       Print object information for each object provided on stdin. May not be
>> +       combined with any other options or arguments except --textconv, --filters
>> +       or --use-mailmap.
>> +       +
>> +       * When used with `--textconv` or `--filters`, the input lines must
>> +        specify the path, separated by whitespace. See the section
>> +        `BATCH OUTPUT` below for details.
>> +       +
>> +       * When used with `--use-mailmap`, the info command shows the size
>> +         of the object as if the identities recorded in it were replaced
>> +         by the mailmap mechanism.
> 
> Same remark about the `info` command as above.
> 
> As the `--batch-check` option prints only object information (which
> includes the size), I think something like the following should be
> used here:
> 
>         * When used with `--use-mailmap`, for commit and tag objects, the
>            printed object information shows the size of the object as if the
>            identities recorded in it were replaced by the mailmap mechanism.
> 
>>   --batch-command::
>>   --batch-command=<format>::
>>          Enter a command mode that reads commands and arguments from stdin. May
>> -       only be combined with `--buffer`, `--textconv` or `--filters`. In the
>> -       case of `--textconv` or `--filters`, the input lines also need to specify
>> -       the path, separated by whitespace. See the section `BATCH OUTPUT` below
>> -       for details.
>> +       only be combined with `--buffer`, `--textconv`, `--use-mailmap` or
>> +       `--filters`.
>> +       +
>> +       * When used with `--textconv` or `--filters`, the input lines must
>> +         specify the path, separated by whitespace. See the section
>> +         `BATCH OUTPUT` below for details.
>> +       +
>> +       * When used with `--use-mailmap`, the info command shows the size
>> +         of the object as if the identities recorded in it were replaced
>> +         by the mailmap mechanism.
> 
> s/info command/`info` command/
> 
> And I think it could be interesting to add:
> 
>    - that it happens only for commits and tags, and
>    - that when the `contents` command is used the idents are replaced.
> 
> So maybe something like:
> 
>         * When used with `--use-mailmap`, for commit and tag objects, the
>           `contents` command shows the identities replaced using the
>            mailmap mechanism, while the `info` command shows the size
>            of the object as if it actually recorded the replacement
>            identities.
> 
> Also this will become true only after your other patch series will be
> merged. Which means that you should say somewhere that either
> 
>    - this patch should be merged only after the other patch series is merged, or
>    - it's a bug that the `info` command doesn't show the size of the
> updated object and this is going to be fixed soon by the other patch
> series anyway.
> 
> Thanks for working on finishing this!
> Christian.