Re: [RFC/PATCH 2/2] builtin-merge: show user-friendly error messages for fast-forward too.

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 30, 2009, 02:23 UTC
Message-ID: <7vy6lo7nu8.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1259497113-1393-3-git-send-email-Matthieu.Moy@imag.fr>

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> This is where I'm not 100% sure I'm not breaking some plumbing.

I think this change is safe for the current codebase, as the only caller of checkout_fast_forward() is cmd_merge(), which is an implementation of "git merge". Even if there were a plumbing implementation that internally runs "git merge", either by calling cmd_merge() or via run_command() interface (there isn't as far as I know), or if somebody adds such a thing in the future, such a "plumbing" will get Porcelain messages from other codepaths in cmd_merge() anyway. You are not introducing a new problem.

Or did you have something a lot more subtle in mind?

Side note. checkout_fast_forward() switches between two commits (the one that matches HEAD) to another (given as remote), which is the same as what "git checkout other-branch" and "git checkout -b new-branch" do. We might want to replace it with the main part of merge_working_tree() from builtin-checkout.c eventually, which would teach the "checkout -m" logic that carries the local changes forward to fast-forward of "git merge".

Show 15 quoted lines

>  builtin-merge.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-merge.c b/builtin-merge.c
> index 57eedd4..0dd363f 100644
> --- a/builtin-merge.c
> +++ b/builtin-merge.c
> @@ -656,6 +656,7 @@ static int checkout_fast_forward(unsigned char *head, unsigned char *remote)
>  	opts.verbose_update = 1;
>  	opts.merge = 1;
>  	opts.fn = twoway_merge;
> +	opts.msgs = get_porcelain_error_msgs();
>  
>  	trees[nr_trees] = parse_tree_indirect(head);
>  	if (!trees[nr_trees++])