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

["René Scharfe" <l.s.r@web.de>]

Am 19.10.22 um 21:41 schrieb Junio C Hamano:
> Jeff King <peff@peff.net> writes:
>
>> That said, I am not too sad to have both. I would not have bothered to
>> do the work to remove the _v() versions with flags. But since René
>> already did so...
>
> It makes two of us ;-)

I wrote that patch out of curiosity and was pleased by the total line
count reduction, getting rid of the on-stack array construction sites
with their magic numbers, not having to manage strvecs explicitly
anymore and the callers staying readable.

One weak spot is the new helper builtin/gc.c::run_git_or_die() that I
added because it was easier than replacing all those strvecs that are
prepared before deciding whether their commands are even needed.

Stripping down the central API to a single shared object (a struct and
functions that get it passed) simplifies it for programmers.  It takes
the idea of d3b2159712 (run-command API: remove "argv" member, always
use "args", 2021-11-25) and c7c4bdeccf (run-command API: remove "env"
member, always use "env_array", 2021-11-25) to its logical conclusion
of going fully dynamic and using standard strvec functions everywhere.
Local shortcuts like builtin/gc.c::run_git_or_die() may still be
defensible.

But still: Is all of that code churn worth it?  Not sure.

René