Re: What's cooking in git.git (Sep 2011, #04; Mon, 12)

From: Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil>
Date: Sep 13, 2011, 15:15 UTC
Message-ID: <DJ8J88If0KG_BPR1BK-feQRQ5yjv1AIYHW3sMosJzbrjCLKCIJABAFuSAH9IrkEl-y3-8mIKsysdqADcV6A7C7f4UuM9aZNpztbz_L9pXXw@cipher.nrlssc.navy.mil>
In-Reply-To: <20110912233348.GE28994@sigill.intra.peff.net>

On 09/12/2011 06:33 PM, Jeff King wrote:

Show 19 quoted lines

> On Mon, Sep 12, 2011 at 04:25:31PM -0700, Junio C Hamano wrote:
> 
>>> Peff also asked if uppercase extensions are common on Windows. They
>>> are, so one often sees .HTM, .HTML, etc. Should this issue be handled
>>> by jk/default-attr?
>>
>> I do not think we would mind adding .HTM but would people limit themselves
>> to uppercase while not limiting themselves to three letters and use .HTML?
> 
> I wonder if they should all be in the style of:
> 
>   [Hh][Tt][Mm][Ll]
>   [Jj][Aa][Vv][Aa]
> 
> for case-challenged systems. That feels like the wrong solution, though.
> If you're on a case-insensitive system, shouldn't we perhaps be
> comparing some kind of canonical version of the filename that is
> lowercased? That would help these built-in attributes, as well as ones
> that people write.

Perhaps fnmatch should be using FNM_CASEFOLD when core.ignorecase is set. We already provide compat/fnmatch/ and have NO_FNMATCH_CASEFOLD in Makefile, so it should be safe to use.

...and I see there is already an fnmatch_icase() in dir.c which adds FNM_CASEFOLD when the global var ignore_case is set. So, maybe it's as easy as:

diff --git a/attr.c b/attr.c
index 09cb4fc..b482262 100644
--- a/attr.c
+++ b/attr.c
@@ -2,6 +2,7 @@
 #include "cache.h"
 #include "exec_cmd.h"
 #include "attr.h"
+#include "dir.h"
 
 const char git_attr__true[] = "(builtin)true";
 const char git_attr__false[] = "\0(builtin)false";
@@ -649,7 +650,7 @@ static int path_matches(const char *pathname, int pathlen,
                /* match basename */
                const char *basename = strrchr(pathname, '/');
                basename = basename ? basename + 1 : pathname;
-               return (fnmatch(pattern, basename, 0) == 0);
+               return (fnmatch_icase(pattern, basename, 0) == 0);
        }
        /*
         * match with FNM_PATHNAME; the pattern has base implicitly
@@ -663,7 +664,7 @@ static int path_matches(const char *pathname, int pathlen,
                return 0;
        if (baselen != 0)
                baselen++;
-       return fnmatch(pattern, pathname + baselen, FNM_PATHNAME) == 0;
+       return fnmatch_icase(pattern, pathname + baselen, FNM_PATHNAME) == 0;
 }
 
 static int macroexpand_one(int attr_nr, int rem);


> Or maybe that is too large a can of worms to open. I sort of assume we
> have those canonicalization routines somewhere already, though.

> 
> I think we're missing Brandon's note that ".F" is used (as distinct from
> ".f", even on case-sensitive filesystems, as it has some magic meaning).
> And the pascal ones somebody mentioned.

Yep, that's all that is missing from my perspective.

-Brandon