From: Jeff King <peff@peff.net>
To: Joshua Jensen <jjensen@workspacewhiz.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] Added support for core.ignorecase when excluding gitignore entries
Date: Thu, 16 Jul 2009 05:42:10 -0400 [thread overview]
Message-ID: <20090716094210.GC2800@coredump.intra.peff.net> (raw)
In-Reply-To: <4A5EB849.1000803@workspacewhiz.com>
On Wed, Jul 15, 2009 at 11:19:05PM -0600, Joshua Jensen wrote:
> This patch allows core.ignorecase=true to work properly with
> gitignore exclusions.
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.
That seems like the right way forward to fixing these problems in the
long term, but it may make sense to do something like your patch in the
meantime.
> - !strcmp(exclude + 1, pathname + pathlen -
> x->patternlen + 1))
> - return to_exclude;
> + if (ignore_case) {
> + if (x->patternlen - 1 <= pathlen &&
> + !strcasecmp(exclude + 1, pathname +
> pathlen - x->patternlen + 1))
> + return to_exclude;
> + } else {
> + if (x->patternlen - 1 <= pathlen &&
> + !strcmp(exclude + 1, pathname + pathlen
> - x->patternlen + 1))
> + return to_exclude;
> + }
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.
-Peff
next prev parent reply other threads:[~2009-07-16 9:42 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-16 5:19 [PATCH] Added support for core.ignorecase when excluding gitignore entries Joshua Jensen
2009-07-16 9:42 ` Jeff King [this message]
2009-07-16 13:15 ` Joshua Jensen
2009-07-20 15:37 ` Jeff King
2009-07-21 15:55 ` Joshua Jensen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090716094210.GC2800@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=jjensen@workspacewhiz.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).