{"thread":{"id":"20130","subject":"[PATCH] Added support for core.ignorecase when excluding gitignore entries","startedAt":"2009-07-16T05:19:05Z","lastAt":"2009-07-21T15:55:17Z","messageCount":5,"participants":["Joshua Jensen","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"118075","messageId":"4A5EB849.1000803@workspacewhiz.com","threadId":"20130","inReplyTo":null,"subject":"[PATCH] Added support for core.ignorecase when excluding gitignore entries","fromName":"Joshua Jensen","fromEmail":"jjensen@workspacewhiz.com","sentAt":"2009-07-16T05:19:05Z","receivedAt":"2009-07-16T05:19:05Z","isPatch":true,"sender":{"key":"jjensen@workspacewhiz.com","avatar":"https://avatars.githubusercontent.com/u/111687?v=4"},"body":"This patch allows core.ignorecase=true to work properly with gitignore \nexclusions.\n\nThis is especially beneficial when using Windows and Perforce and the \ngit-p4 bridge.  Perforce preserves a given file's full path including \ncase.  When syncing a file down, directories are created, if necessary, \nusing the case as stored with the filename.  Unfortunately, two files in \nthe same directory can have differing cases for their respective paths, \nsuch as /dirA/file1.c and /DirA/file2.c.  Depending on sync order, DirA/ \nmay get created instead of dirA/.\n\nTrying to catch all case combinations for a set of gitignore entries is \nvery difficult.  Having the exclusions honor the core.ignorecase=true \nmakes the process less error prone.\n---\n dir.c |   49 ++++++++++++++++++++++++++++++++++++-------------\n 1 files changed, 36 insertions(+), 13 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 0e6b752..c63e0a0 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -315,14 +315,25 @@ static int excluded_1(const char *pathname,\n             if (x->flags & EXC_FLAG_NODIR) {\n                 /* match basename */\n                 if (x->flags & EXC_FLAG_NOWILDCARD) {\n-                    if (!strcmp(exclude, basename))\n-                        return to_exclude;\n+                    if (ignore_case) {\n+                        if (!strcasecmp(exclude, basename))\n+                            return to_exclude;\n+                    } else {\n+                        if (!strcmp(exclude, basename))\n+                            return to_exclude;\n+                    }\n                 } else if (x->flags & EXC_FLAG_ENDSWITH) {\n-                    if (x->patternlen - 1 <= pathlen &&\n-                        !strcmp(exclude + 1, pathname + pathlen - \nx->patternlen + 1))\n-                        return to_exclude;\n+                    if (ignore_case) {\n+                        if (x->patternlen - 1 <= pathlen &&\n+                            !strcasecmp(exclude + 1, pathname + pathlen \n- x->patternlen + 1))\n+                            return to_exclude;\n+                    } else {\n+                        if (x->patternlen - 1 <= pathlen &&\n+                            !strcmp(exclude + 1, pathname + pathlen - \nx->patternlen + 1))\n+                            return to_exclude;\n+                    }\n                 } else {\n-                    if (fnmatch(exclude, basename, 0) == 0)\n+                    if (fnmatch(exclude, basename, ignore_case ? \nFNM_CASEFOLD : 0) == 0)\n                         return to_exclude;\n                 }\n             }\n@@ -335,17 +346,29 @@ static int excluded_1(const char *pathname,\n                 if (*exclude == '/')\n                     exclude++;\n \n-                if (pathlen < baselen ||\n-                    (baselen && pathname[baselen-1] != '/') ||\n-                    strncmp(pathname, x->base, baselen))\n-                    continue;\n+                if (ignore_case) {\n+                    if (pathlen < baselen ||\n+                        (baselen && pathname[baselen-1] != '/') ||\n+                        strncasecmp(pathname, x->base, baselen))\n+                        continue;\n+                } else {\n+                    if (pathlen < baselen ||\n+                        (baselen && pathname[baselen-1] != '/') ||\n+                        strncmp(pathname, x->base, baselen))\n+                        continue;\n+                }\n \n                 if (x->flags & EXC_FLAG_NOWILDCARD) {\n-                    if (!strcmp(exclude, pathname + baselen))\n-                        return to_exclude;\n+                    if (ignore_case) {\n+                        if (!strcasecmp(exclude, pathname + baselen))\n+                            return to_exclude;\n+                    } else {\n+                        if (!strcmp(exclude, pathname + baselen))\n+                            return to_exclude;\n+                    }\n                 } else {\n                     if (fnmatch(exclude, pathname+baselen,\n-                            FNM_PATHNAME) == 0)\n+                            FNM_PATHNAME | (ignore_case ? FNM_CASEFOLD \n: 0)) == 0)\n                         return to_exclude;\n                 }\n             }\n-- \n1.6.3.2.1299.gee46c.dirty\n"},{"id":"118104","messageId":"20090716094210.GC2800@coredump.intra.peff.net","threadId":"20130","inReplyTo":"4A5EB849.1000803@workspacewhiz.com","subject":"Re: [PATCH] Added support for core.ignorecase when excluding gitignore entries","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-07-16T09:42:10Z","receivedAt":"2009-07-16T09:42:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 15, 2009 at 11:19:05PM -0600, Joshua Jensen wrote:\n\n> This patch allows core.ignorecase=true to work properly with\n> gitignore exclusions.\n\nMakes sense, though I can't help but wonder what would happen with a\nfilesystem that did more than just case (like the utf8 normalization\nthat happens on HFS).\n\nShould we actually be converting the filesystem names into a canonical\nformat as they are read? IIRC, Linus posted some patches a few weeks ago\nabout \"git path\" versus \"filesystem path\", but I didn't actually look\ntoo closely.\n\nThat seems like the right way forward to fixing these problems in the\nlong term, but it may make sense to do something like your patch in the\nmeantime.\n\n> -                        !strcmp(exclude + 1, pathname + pathlen -\n> x->patternlen + 1))\n> -                        return to_exclude;\n> +                    if (ignore_case) {\n> +                        if (x->patternlen - 1 <= pathlen &&\n> +                            !strcasecmp(exclude + 1, pathname +\n> pathlen - x->patternlen + 1))\n> +                            return to_exclude;\n> +                    } else {\n> +                        if (x->patternlen - 1 <= pathlen &&\n> +                            !strcmp(exclude + 1, pathname + pathlen\n> - x->patternlen + 1))\n> +                            return to_exclude;\n> +                    }\n\nIf your patch is the right route, it might be nice to collapse the\ncomparison into its own function. You end up cutting and pasting a lot\nof the related conditionals and returns (like above, where 2 lines\nbecome 9), so it might make sense to do something like:\n\n  int filename_cmp(const char *a, const char *b, int ignore_case)\n  {\n    return ignore_case ? strcasecmp(a, b) : strcmp(a, b);\n  }\n\nand then just s/strcmp/filename_cmp/ at the appropriate callsites.\n\n-Peff\n"},{"id":"118112","messageId":"4A5F27EE.3070101@workspacewhiz.com","threadId":"20130","inReplyTo":"20090716094210.GC2800@coredump.intra.peff.net","subject":"Re: [PATCH] Added support for core.ignorecase when excluding gitignore entries","fromName":"Joshua Jensen","fromEmail":"jjensen@workspacewhiz.com","sentAt":"2009-07-16T13:15:26Z","receivedAt":"2009-07-16T13:15:26Z","isPatch":true,"sender":{"key":"jjensen@workspacewhiz.com","avatar":"https://avatars.githubusercontent.com/u/111687?v=4"},"body":"----- Original Message -----\nFrom: Jeff King\nDate: 7/16/2009 3:42 AM\n> Makes sense, though I can't help but wonder what would happen with a\n> filesystem that did more than just case (like the utf8 normalization\n> that happens on HFS).\n>\n> Should we actually be converting the filesystem names into a canonical\n> format as they are read? IIRC, Linus posted some patches a few weeks ago\n> about \"git path\" versus \"filesystem path\", but I didn't actually look\n> too closely.\n>   \nI'm game for whatever.  Git actually has a lot of places where it \ndoesn't pay attention to core.ignorecase, and having a standard and \ncorrect method of comparing filenames would make it easier to handle \ncore.ignorecase=true in a more global fashion.\n> If your patch is the right route, it might be nice to collapse the\n> comparison into its own function. You end up cutting and pasting a lot\n> of the related conditionals and returns (like above, where 2 lines\n> become 9), so it might make sense to do something like:\n>\n>   int filename_cmp(const char *a, const char *b, int ignore_case)\n>   {\n>     return ignore_case ? strcasecmp(a, b) : strcmp(a, b);\n>   }\n>\n> and then just s/strcmp/filename_cmp/ at the appropriate callsites.\n>   \nI started off with this method, but it required two functions, one with \nthe strcmp() and one for strncmp().  In fact, in other places in the \ncode, Git uses memcmp() for comparison.  Is that, then, three filename \ncomparison functions, dependent upon intent?  At that point, it felt \nlike my change wasn't as self contained anymore, so I then wrote what I \nposted to the list to get feedback.\n\nI'm hoping someone will offer the most correct method to do this, as I \nhave a number of patches forthcoming to handle core.ignorecase=true in \nother areas.  The next one covers 'git status' and its reporting of \n'missing' directories due to case differences.\n\nThanks!\n\nJosh\n"},{"id":"118327","messageId":"20090720153737.GF5347@coredump.intra.peff.net","threadId":"20130","inReplyTo":"4A5F27EE.3070101@workspacewhiz.com","subject":"Re: [PATCH] Added support for core.ignorecase when excluding gitignore entries","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-07-20T15:37:37Z","receivedAt":"2009-07-20T15:37:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 16, 2009 at 07:15:26AM -0600, Joshua Jensen wrote:\n\n> >Should we actually be converting the filesystem names into a canonical\n> >format as they are read? IIRC, Linus posted some patches a few weeks ago\n> >about \"git path\" versus \"filesystem path\", but I didn't actually look\n> >too closely.\n> I'm game for whatever.  Git actually has a lot of places where it\n> doesn't pay attention to core.ignorecase, and having a standard and\n> correct method of comparing filenames would make it easier to handle\n> core.ignorecase=true in a more global fashion.\n\nLike I said, I'm not sure what the status of that is, so probably\nsomething simple like your patch makes sense in the interim (unless we\nhear from somebody more clueful).\n\n> >If your patch is the right route, it might be nice to collapse the\n> >comparison into its own function. You end up cutting and pasting a lot\n> >of the related conditionals and returns (like above, where 2 lines\n> >become 9), so it might make sense to do something like:\n> >\n> >  int filename_cmp(const char *a, const char *b, int ignore_case)\n> >  {\n> >    return ignore_case ? strcasecmp(a, b) : strcmp(a, b);\n> >  }\n> >\n> >and then just s/strcmp/filename_cmp/ at the appropriate callsites.\n> I started off with this method, but it required two functions, one\n> with the strcmp() and one for strncmp().  In fact, in other places in\n> the code, Git uses memcmp() for comparison.  Is that, then, three\n> filename comparison functions, dependent upon intent?  At that point,\n> it felt like my change wasn't as self contained anymore, so I then\n> wrote what I posted to the list to get feedback.\n\nIMHO, you are better off even with three wrapper functions, just because\nthey are all very straightforward. Whereas with your patch, I felt like\nthe innards of complex functions got harder to read because of big\nduplicate conditionals. But that's just my two cents.\n\n-Peff\n"},{"id":"118383","messageId":"4A65E4E5.3030709@workspacewhiz.com","threadId":"20130","inReplyTo":"20090720153737.GF5347@coredump.intra.peff.net","subject":"Re: [PATCH] Added support for core.ignorecase when excluding gitignore entries","fromName":"Joshua Jensen","fromEmail":"jjensen@workspacewhiz.com","sentAt":"2009-07-21T15:55:17Z","receivedAt":"2009-07-21T15:55:17Z","isPatch":true,"sender":{"key":"jjensen@workspacewhiz.com","avatar":"https://avatars.githubusercontent.com/u/111687?v=4"},"body":"----- Original Message -----\nFrom: Jeff King\nDate: 7/20/2009 9:37 AM\n>>> If your patch is the right route, it might be nice to collapse the\n>>> comparison into its own function. You end up cutting and pasting a lot\n>>> of the related conditionals and returns (like above, where 2 lines\n>>> become 9), so it might make sense to do something like:\n>>>\n>>>  int filename_cmp(const char *a, const char *b, int ignore_case)\n>>>  {\n>>>    return ignore_case ? strcasecmp(a, b) : strcmp(a, b);\n>>>  }\n>>>\n>>> and then just s/strcmp/filename_cmp/ at the appropriate callsites.\n>>>       \n> IMHO, you are better off even with three wrapper functions, just because\n> they are all very straightforward. Whereas with your patch, I felt like\n> the innards of complex functions got harder to read because of big\n> duplicate conditionals. But that's just my two cents.\n>   \nI agree.  I will update the patch soon.\n\nJosh\n"}]}