Re: [PATCH] Added support for core.ignorecase when excluding gitignore entries

From: Joshua Jensen <jjensen@workspacewhiz.com>
Date: Jul 16, 2009, 13:15 UTC
Message-ID: <4A5F27EE.3070101@workspacewhiz.com>
In-Reply-To: <20090716094210.GC2800@coredump.intra.peff.net>

----- Original Message -----
From: Jeff King
Date: 7/16/2009 3:42 AM

Show 9 quoted lines

> Makes sense, though I can't help but wonder what would happen with a
> filesystem that did more than just case (like the utf8 normalization
> that happens on HFS).
>
> Should we actually be converting the filesystem names into a canonical
> format as they are read? IIRC, Linus posted some patches a few weeks ago
> about "git path" versus "filesystem path", but I didn't actually look
> too closely.
>

I'm game for whatever. Git actually has a lot of places where it doesn't pay attention to core.ignorecase, and having a standard and correct method of comparing filenames would make it easier to handle core.ignorecase=true in a more global fashion.

Show 12 quoted lines

> If your patch is the right route, it might be nice to collapse the
> comparison into its own function. You end up cutting and pasting a lot
> of the related conditionals and returns (like above, where 2 lines
> become 9), so it might make sense to do something like:
>
>   int filename_cmp(const char *a, const char *b, int ignore_case)
>   {
>     return ignore_case ? strcasecmp(a, b) : strcmp(a, b);
>   }
>
> and then just s/strcmp/filename_cmp/ at the appropriate callsites.
>

I started off with this method, but it required two functions, one with the strcmp() and one for strncmp(). In fact, in other places in the code, Git uses memcmp() for comparison. Is that, then, three filename comparison functions, dependent upon intent? At that point, it felt like my change wasn't as self contained anymore, so I then wrote what I posted to the list to get feedback.

I'm hoping someone will offer the most correct method to do this, as I have a number of patches forthcoming to handle core.ignorecase=true in other areas. The next one covers 'git status' and its reporting of 'missing' directories due to case differences.

Thanks!

Josh