Re: [PATCH v2] builtin: allow passing custom data to sub-commands

From: karthik nayak <karthik.188@gmail.com>
Date: Nov 26, 2024, 16:41 UTC
Message-ID: <CAOLa=ZTeRUT6Ex5Pa-fM1i-QbTfnmDN=o=MU_N2VFeLQwbscgg@mail.gmail.com>
In-Reply-To: <Z0W3rgHQhmUxjgfp@ArchLinux>

shejialuo <shejialuo@gmail.com> writes:

Show 17 quoted lines

> On Tue, Nov 26, 2024 at 12:19:22PM +0100, Karthik Nayak wrote:
>> In 9b1cb5070f (builtin: add a repository parameter for builtin
>> functions, 2024-09-13) the repository was passed down to all builtin
>> commands. This allowed the repository to be passed down to lower layers
>> without depending on the global `the_repository` variable.
>>
>> To remove usage of global variables like `the_repository` in
>> sub-commands, it makes sense to pass down the repository value from the
>> commands to the sub-commands. But let's make it more generic and modify
>> `parse_opt_subcommand_fn` to instead take a `void *` value. This way we
>> can provide custom structures to each sub-command.
>>
>
> From my perspective, I think using either "struct repository *" or "void
> *" is OK. However, I am a little concerned about using "void *" at
> current. It gives me a feeling that we over-optimize here.
>

Could you elaborate on why you think this is an over-optimization? We don't loose functionality with this, nor do we have to have add additional code to handle the typecasting to `struct repository *` if needed. But would definitely like to resolve anything I missed.