Subject: Re: [PATCH 05/13] delete_refs(): improve error message

From: Junio C Hamano <[email-masked]>
Date: Tue, 09 Jun 2015 11:47:56 -0700

Michael Haggerty <[email-masked]> writes:

> Change the error message from
>
>     Could not remove branch %s
>
> to
>
>     Could not remove reference %s
>
> This change makes sense even for the existing caller, which uses the
> function to delete remote-tracking branches.

I am 80% convinced ;-)

The existing caller never used this for removing tags, so 'could not
remove branch' was equally correct for it and was more specific than
'could not remove reference'.  If you change it to 'could not remove
that thing %s', it would still be correct for the existing caller;
it would be even less specific for them, though ;-)

The new callers you will add in later patch of course cannot live
with 'could not remove branch', so I think that this is an
acceptable compromise we can live with.  If somebody later wants to
make the message more specific, they can add code that switches on
the prefix of the ref when coming up with the error message (and use
that code consistently in other error messages e.g. 'could not add
reference').

>
> Signed-off-by: Michael Haggerty <[email-masked]>
> ---
>  refs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/refs.c b/refs.c
> index c413282..2a2a06d 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2827,7 +2827,7 @@ int delete_refs(struct string_list *refnames)
>  		const char *refname = refnames->items[i].string;
>  
>  		if (delete_ref(refname, NULL, 0))
> -			result |= error(_("Could not remove branch %s"), refname);
> +			result |= error(_("Could not remove reference %s"), refname);
>  	}
>  
>  	return result;