git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jeff King <peff@peff.net>
Date
Jul 20, 2009, 15:37 UTC
Message-ID
<20090720153737.GF5347@coredump.intra.peff.net>
In-Reply-To
<4A5F27EE.3070101@workspacewhiz.com>
On Thu, Jul 16, 2009 at 07:15:26AM -0600, Joshua Jensen wrote:
Show 8 quoted lines
> >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.

Like I said, I'm not sure what the status of that is, so probably something simple like your patch makes sense in the interim (unless we hear from somebody more clueful).

Show 17 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.

IMHO, you are better off even with three wrapper functions, just because they are all very straightforward. Whereas with your patch, I felt like the innards of complex functions got harder to read because of big duplicate conditionals. But that's just my two cents.

-Peff
Previous: Joshua JensenNext: Joshua Jensen
Message 4 of 5 in “Added support for core.ignorecase when excluding gitignore entries”
  1. Added support for core.ignorecase when excluding gitignore entriesJoshua Jensen, Jul 16, 2009
  2. Jeff KingJul 16, 2009
  3. Joshua JensenJul 16, 2009
  4. Jeff KingJul 20, 2009
  5. Joshua JensenJul 21, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.