Re: [PATCH 0/8] run-command: remove run_command_v_*()

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Oct 28 2022, René Scharfe wrote:

> Am 27.10.22 um 23:46 schrieb Ævar Arnfjörð Bjarmason:
>>
>> - I wish C had a nicer syntax for not just declaring but squashing
>>   together compile_time bracketed lists (think set operations). But the
>>   below "CHILD_PROCESS_INIT_LIST" is a pretty good poor man's version.
>>
>>   I see gcc/clang nicely give us safety rails for that with
>>   "-Woverride-init", for this sort of "opts struct with internal stuff,
>>   but also user options" I think it works out very nicely.
>>
>
> That's a nice and simple macro.  I played with a gross variant à la
>
>   #define CHILD_PROCESS_INIT_EX(...) { .args = STRVEC_INIT, __VA_ARGS__ }
>
> which would allow e.g.
>
>   struct child_process cmd = CHILD_PROCESS_INIT_EX(.git_cmd = 1);
>
> Yours is better, 

I actually think yours is better, anyway...

> but they share the downside of not actually saving any lines of code..

To me it's not about saving code, but that it's immediately obvious when
reading the code that this set of options can be determined and set at
function or scope entry.

We tend to otherwise have creep where the decl and option init drifts
apart over time, and with complex init's you might stare at it for 30s,
before realizing that between the decl and fully init ing it often 50
lines later nothing actually changed vis-a-vis the state, we could have
just done it earlier.

I think that's worth it in general, whether it's worth the churn in this
case...

>> - We have quite a few callers that want "on error, die", so maybe we
>>   should have something like that "on_error" sooner than later.
>
> We could add a die_on_error bit for that, or start_command_or_die() and
> run_command_or_die() variants (there I go again, multiplying APIs..).
> They could report the failed command, which a caller can't do because
> the internal strvec is already cleared once it learns of the failure.

*nod*