Re: [PATCHv4 3/5] run-command: add {run,start,finish}_command_or_die

[Stefan Beller <sbeller@google.com>]

On Tue, Dec 20, 2016 at 10:33 AM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 20.12.2016 um 00:28 schrieb Stefan Beller:
>>
>> +static void report_and_die(struct child_process *cmd, const char *action)
>> +{
>> +       int i;
>> +       struct strbuf err = STRBUF_INIT;
>> +       if (cmd->git_cmd)
>> +               strbuf_addstr(&err, "git ");
>> +       for (i = 0; cmd->argv[i]; )
>> +               strbuf_addf(&err, "'%s'", cmd->argv[i]);
>
>
> Take note that cmd is accessed here.
>
>> +       die(_("could not %s %s"), action, err.buf);
>
>
> Should lego sentences not be avoided? They are not exactly translator
> friendly.
>
> Given that a lot of effort is spent elsewhere to actually *avoid* dying in
> library code, this new die() is not very welcome, I must say.

I agree on the sentiment. In a reroll I'll drop this patch and instead introduce
a function `char *get_child_command_line(struct child_process*);`, which
a caller can call before calling finish_command and then use the
resulting string
to assemble an error message without lego.

Thanks for the thorough review!
Stefan