From: Thomas Adam <thomas@xteddy.org>
Date: Sun, 03 Oct 2010 21:59:10 GMT
Subject: Re: [PATCH/RFC v3 6/8] Add case insensitivity support when using git ls-files
Message-ID: <AANLkTimRa09+nBFTV9OtzKngAb=QrAP550a22S73cW_y@mail.gmail.com>
In-Reply-To: <201010032019.09244.j6t@kdbg.org>

Hi --

On 3 October 2010 19:19, Johannes Sixt <j6t@kdbg.org> wrote:
> On Sonntag, 3. Oktober 2010, Thomas Adam wrote:
>> Hi --
>>
>> On 3 October 2010 10:56, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> > +       if (ignore_case) {
>> > +               for (;;) {
>> > +                       unsigned char c1 = tolower(*match);
>> > +                       unsigned char c2 = tolower(*name);
>> > +                       if (c1 == '\0' || is_glob_special(c1))
>> > +                               break;
>> > +                       if (c1 != c2)
>> > +                               return 0;
>> > +                       match++;
>> > +                       name++;
>> > +                       namelen--;
>> > +               }
>> > +       } else {
>> > +               for (;;) {
>> > +                       unsigned char c1 = *match;
>> > +                       unsigned char c2 = *name;
>> > +                       if (c1 == '\0' || is_glob_special(c1))
>> > +                               break;
>> > +                       if (c1 != c2)
>> > +                               return 0;
>> > +                       match++;
>> > +                       name++;
>> > +                       namelen--;
>> > +               }
>> >        }
>>
>> It's a real shame about the code duplication here.  Can we not avoid
>> it just by doing:
>>
>> unsigned char c1 = (ignore_case) ? tolower(*match) : *match;
>> unisgned char c2 = (ignore_case) ? tolower(*name) : *name;
>>
>> I appreciate that to some it might look like perl golf, but...
>
> It has been discussed, and IIRC, the concensus was to keep the code
> duplication because this is an inner loop.

I must have missed the discussion -- but why/how does making it an
inner-loop somehow prevent it from such an obvious (and readable)
optimisation, which would have fitted in well in other areas.

-- Thomas Adam