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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> I do think Junio saying "consensus" may have been premature. I expressed
> my opinion and he agreed, but I think that is as far as it got. :)

Maybe.  This is a tangent, but as far as I am concerned, when Réne
writes something that looks to me very straight-forward and correct,
and it passes your taste buds, then that is enough consensus to move
ahead.  As Linus often said and I concur, some people got good
taste, that is hard to quantify and probably hard to teach, and
there are a handful folks here with good taste.  And when two who
have demonstrated they are with good taste agrees, that is good
enough to me.

>> I don't see how *_l_opt() is particularly error prone, I just had a
>> stupid think-o in v1 of this, but that if/else if bug is something that
>> could have snuck in with run_command() given the same stupidity :)
>
> I don't think it's error-prone. It just seems like it complicates an API
> for little gain, and causes us to have a lot of boilerplate mapping
> RUN_* flags into cmd.* fields.

True. run_command() needs the RUN_* flags twiddling, too, so it is
not a point against _l_opt() variant.  What I see as its biggest
problem is it is a bit too rigid for many of the current callers
that use _v_opt() to replace them, and can easily tempt developers
to write almost-duplicate _l_opt() calls, leading to an easily
avoidable bug like we saw in the if/else if/else cascade.

Thanks.