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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 1, 2010, 14:59 UTC
Message-ID: <7vsk1t7bn2.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1283321153-24745-1-git-send-email-Matthieu.Moy@imag.fr>

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.

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.

The function is not about setting the error messages anymore (and it sounds like it never has been since the current behaviour was introduced) but is about declaring that we are a Porcelain and won't stop at the first error. We might end up introducing different behaviours later in this function.

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?