Re: [PATCH v2 05/12] delete_refs(): improve error message

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Jun 19, 2015, 14:20 UTC
Message-ID: <5584253E.9030001@alum.mit.edu>
In-Reply-To: <xmqq7fr4akr7.fsf@gitster.dls.corp.google.com>

On 06/15/2015 08:29 PM, Junio C Hamano wrote:

Show 26 quoted lines

> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> Subject: Re: [PATCH v2 05/12] delete_refs(): improve error message
> 
> I'd call this "make error message more generic".
> 
>> Change the error message from
>>
>>     Could not remove branch %s
>>
>> to
>>
>>     could not remove reference %s
>>
>> * s/branch/reference/. This change makes sense even for the existing
>>   caller, which uses the function to delete remote-tracking
>>   branches.
> 
> and replace this bullet with something like:
> 
> * Originally 'branch -d' was the only caller of this, but as part
>   of the refs API, we will allow it to be called on refs that is not
>   a branch or a remote-tracking branch.
> 
> as calling a remote-tracking branch a 'branch' is not incorrect
> per-se.

True, but "branch refs/remotes/origin/foo" is not how we would usually refer to that reference. Usually we would say "reference refs/remotes/origin/foo" or "remote-tracking branch origin/foo". But I will make approximately the change that you suggest.

Show 8 quoted lines

> What would count as true improvement is ...
> 
>> * Convert it to lower case, as per our usual convention.
> 
> ... this one.  If somebody eventually chooses to make the message
> finer grained by switching on the prefix refs/{what}, so that the
> user can differentiate between branches, remote-tracking branches,
> tags, etc., that would also count as improvement as well.

I thought about proposing a function that changes a reference name into its prose equivalent, but I'm pretty sure that the result would not be internationalizable. Probably such a function would have to look like

enum refname_kind { REFNAME_KIND_BRANCH, REFNAME_KIND_TAG, REFNAME_KIND_REMOTE, ...(?), REFNAME_KIND_OTHER};

enum refname_kind shorten_refname(const char *refname, const char **short_refname);

and callers would still need logic to pick the correct (internationalized) formatting string based on the return value of the function.

Someday...

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu