Re: [PATCH 1/5 v2] merge-recursive: porcelain messages for checkout

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Jun 15, 2010, 12:36 UTC
Message-ID: <vpqr5k8zd4z.fsf@bauges.imag.fr>
In-Reply-To: <1276604576-28092-2-git-send-email-diane.gasselin@ensimag.imag.fr>

Diane Gasselin <diane.gasselin@ensimag.imag.fr> writes:

Show 8 quoted lines

> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -372,6 +372,7 @@ static int merge_working_tree(struct checkout_opts *opts,
>  		topts.src_index = &the_index;
>  		topts.dst_index = &the_index;
>  
> +		topts.msgs = get_porcelain_error_msgs("checkout");
>  		topts.msgs.not_uptodate_file = "You have local changes to '%s'; cannot switch branches.";

It's nice to get accurate messages for all cases, but then why do you keep the special-case for not_uptodate_file? If there's a good reason for it, a comment in the code would be welcome.

Show 5 quoted lines

> +	/* would_overwrite */
> +	msgs.would_overwrite = malloc(sizeof(char) * 72);
> +	sprintf((char *)msgs.would_overwrite,
> +		"Your local changes to '%%s' would be overwritten by %s.  Aborting.",
> +		cmd);

This yields:

  Your local changes to 'foo' would be overwritten by checkout.  Aborting.

I tend to prefer Junio's wording:

  You have local changes to 'foo'; cannot switch branches.

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