Re: [PATCH v2 00/10] run-command API: add run_command_{l,sv}_opt()

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 19, 2022, 18:00 UTC
Message-ID: <xmqq7d0vsngp.fsf@gitster.g>
In-Reply-To: <Y1AujKKPUJFepph5@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 7 quoted lines

>> True. run_command() needs the RUN_* flags twiddling, too, so it is
>> not a point against _l_opt() variant.
>
> Did you mean run_command_v() here? If so, then yes, it requires the
> flags. But if we are going to drop it in favor of just run_command(),
> then those flags go away (and moving to the _l() variant is a step in
> the opposite direction).

Ah, but we'd still need to prepare individual bits in the child structure (e.g. RUN_GIT_CMD vs .git_cmd) anyway. Even though we may not have to work with the flags, somehow we need to set it up. So it is not all that strong argument against the _l_opt().

The above assessment is primarily because I do not have too much against RUN_GIT_CMD and friends, compared to setting the individual members in the child_process struct. Setting individual members in the struct is more direct and it may be an improvement use run_command() directly over using _v_opt() and _l_opt() variants.