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

From: Stefan Beller <sbeller@google.com>
Date: Dec 20, 2016, 19:23 UTC
Message-ID: <CAGZ79kYNKWfnEXWJfyRUutFyaQiRD9qW--LkK4Nbwdf7FtdPQA@mail.gmail.com>
In-Reply-To: <aad0af97-7588-632d-a113-5d8372b8b7a8@kdbg.org>

On Tue, Dec 20, 2016 at 10:33 AM, Johannes Sixt <j6t@kdbg.org> wrote:

Show 22 quoted lines

> 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