Re: [PATCH] Make sure show_all_errors when using porcelain error messages

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> opts.show_all_errors controls the number of errors that can be displayed,
>> and set_porcelain_error_msgs gives the format. But the formatting would
>> be incorrect if set_porcelain_error_msgs is called without setting
>> opts.show_all_errors.
>
> That makes it sound like a design bug of set_porcelain_error_msgs(), in
> that the caller _cannot_ choose to stop at the first error if it wants to
> use friendlier message than the plumbing one.

You're right that the caller cannot stop at the first error and get
friendly message, but I don't think this is a bug. As a user, I don't
see any reason not to want the complete list. We kept the
"show_all_errors = false" behavior to allow the slight performance
improvement of stopping at the first error, which may (but I'm not
even sure) benefit to scripts.

> But if that is (and I do not think it is such a bad thing if we decide
> that Porcelains have no option of stopping at the first error) then we
> should set opts.show_all_errors in the set_porcelain_error_msgs()
> function, and probably should rename the function to something
> saner.

Yes (actually, I send a quick patch to make sure this was the problem
of the OP, but the intention was to have a better one when I have
time. I forgot to add RFC in the title).

> How about making it
>
> 	int setup_unpack_trees_porcelain(struct unpack_trees_options *opt,
> 						const char *cmd);
>
> and possibly moving it from merge-recursive.c to unpack-trees.c?

Sounds good, yes. Will do (probably tomorrow).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/