Re: [PATCH 2/2] merge: use help_unknown_ref instead of die

From: Junio C Hamano <gitster@pobox.com>
Date: May 1, 2013, 18:36 UTC
Message-ID: <7vip325yzq.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1367407327-5216-3-git-send-email-vikrant.varma94@gmail.com>

Vikrant Varma <vikrant.varma94@gmail.com> writes:

Show 21 quoted lines

> The previous patch added help_unknown_ref to print a more helpful error 
> message when trying to merge a branch that doesn't exist, by printing a 
> list of remote branches the user might have meant. Use it.
>
> Signed-off-by: Vikrant Varma <vikrant.varma94@gmail.com>
> ---
>  builtin/merge.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 3e2daa3..0f1f39b 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1053,8 +1053,8 @@ static struct commit_list *collect_parents(struct commit *head_commit,
>  		remotes = &commit_list_insert(head_commit, remotes)->next;
>  	for (i = 0; i < argc; i++) {
>  		struct commit *commit = get_merge_parent(argv[i]);
> -		if (!commit)
> -			die(_("%s - not something we can merge"), argv[i]);
> +		if (!commit) 
> +		        help_unknown_ref(argv[i]);

This calling site may become something like:

	if (!commit) {
        	char *suggestion;
                suggestion = guess_misspelled_ref(argv[i]);
                die(suggestion == NULL
                    ? _("%s - not something we can merge")
                    : _("%s - not something we can merge\n"
                        "Perhaps you meant one of these?\n"
                        "%s"), argv[i], suggestion);
	}

if you really want to keep "not something we can merge" at the top.

I however suspect that this might be easier for the reader.

	if (!commit) {
        	struct string_list *suggestion;
                suggestion = guess_misspelled_ref(argv[i]);
                if (suggestion)
			print_string_list(suggestion,
                                      	_("Perhaps you meant one of these?"));
               	die(_("%s - not something we can merge"), argv[i]);
	}

Note that print_string_list() needs to be enhanced so that the caller can tell it not to show the .util field if you go in this direction.

>  		remotes = &commit_list_insert(commit, remotes)->next;
>  	}
>  	*remotes = NULL;

Thanks.