From: René Scharfe <l.s.r@web.de>
Date: Thu, 20 Oct 2022 18:34:10 GMT
Subject: Re: [PATCH v2 00/10] run-command API: add run_command_{l,sv}_opt()
Message-ID: <84971a3a-4c5c-5b79-582e-88b3bd8c624f@web.de>
In-Reply-To: <xmqq5ygfr48b.fsf@gitster.g>

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é