On Tue, Dec 20, 2016 at 10:33 AM, Johannes Sixt <j6t@xxxxxxxx> 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