Re: [PATCH v2 1/4] Refactor skipping DOS drive prefixes

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 11, 2016, 09:32 UTC
Message-ID: <alpine.DEB.2.20.1601111029540.2964@virtualbox>
In-Reply-To: <xmqq8u3z20aj.fsf@gitster.mtv.corp.google.com>

Hi Eric & Junio,

On Fri, 8 Jan 2016, Junio C Hamano wrote:

Show 18 quoted lines

> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > With this change, code such as:
> >
> >     for (i = has_dos_drive_prefix(src); i > 0; i--)
> >         ...
> >
> > in path.c reads a bit oddly. Renaming the function might help. For instance:
> >
> >     for (i = dos_drive_prefix_len(src); i > 0; i--)
> >         ...
> 
> Renaming may be unnecessary churn, but I do not think we mind an
> additional synonym, e.g.
> 
>     #define has_dos_drive_prefix(x) dos_drive_prefix_len(x)
> 
> if some people prefer.

I am actually not so sure about this: if I read `dos_drive_prefix_len(path)` I would have assumed the return value to be -1 if `path` does not, in fact, have a DOS drive prefix.

Sure, returning the length of the DOS drive prefix when just asking whether it has one is a bit surprising at first, but it also makes sense: we already have that information, so we might just as well use it.

In any case, I think this change (if it is really considered desirable) could easily be an add-on patch by people who care about this ;-)

Ciao, Dscho