Re: [PATCH v1 1/1] cat-file: add use-mailmap/no-use-mailmap to --batch-command

[Siddharth Asthana <siddharthasthana31@gmail.com>]

On 29/03/26 06:20, Junio C Hamano wrote:
> Siddharth Asthana <siddharthasthana31@gmail.com> writes:
> 
>> git-cat-file(1)'s --batch-command works with the --use-mailmap option,
>> but this option needs to be set when the process is created. This means
>> we cannot change this option mid-operation.
>>
>> At GitLab, Gitaly caches git-cat-file processes and it would be useful
>> if --batch-command supported toggling mailmap dynamically with existing
>> processes.
>>
>> Add `use-mailmap` and `no-use-mailmap` as new subcommands to
>> --batch-command. `use-mailmap` enables the mailmap mechanism for
>> subsequent `contents` and `info` commands, lazily loading the mailmap
>> data from disk on first invocation. `no-use-mailmap` disables it but
>> keeps the data in memory so re-enabling doesn't require reloading.
> 
> And the cached information in process, if any, would be flushed?


No. The mailmap data is kept in memory. Turning it off only disables 
application of mailmap; turning it back on reuses already loaded data.

> 
> Because in your design, these command words are parsed separately
> from and have nothing in common with the command line options, I
> wonder if it is easier to understand if you added just a single
> command, "mailmap", that takes a single parameter "yes" or "no"?


Agreed, this is cleaner.

I will rerolled this as a single `mailmap` subcommand with `yes|no`, so 
the dispatch table has one entry and the handling is in one function

I'll send v2 with this update.


Thanks,
Siddharth


> Then this hunk would ...
> 
>> +	{ "contents", parse_cmd_contents, 1 },
>> +	{ "info", parse_cmd_info, 1 },
>> +	{ "flush", NULL, 0 },
>> +	{ "use-mailmap", parse_cmd_use_mailmap, 0 },
>> +	{ "no-use-mailmap", parse_cmd_disable_mailmap, 0 },
> 
> ... need only one addition instead of two entries and you do not
> have to write boilerplate-rich helper functions twice.