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

Junio C Hamano [off-list ref] writes:

Matthieu Moy [off-list ref] writes:

quoted

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/