Re: [PATCH 0/8] run-command: remove run_command_v_*()
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Oct 28, 2022, 16:11 UTC
- Message-ID
- <221028.8635b87wo6.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <749f6adc-928a-0978-e3a1-2ede9f07def0@web.de>
On Fri, Oct 28 2022, René Scharfe wrote:
Show 20 quoted lines
> 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...
Show 7 quoted lines
>> - 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*