From: René Scharfe Date: Wed, 14 Nov 2012 19:30:36 GMT Subject: Re: [PATCH nd/wildmatch] Correct Git's version of isprint and isspace Message-ID: <50A3F15C.7000200@lsrfire.ath.cx> In-Reply-To: Am 13.11.2012 20:50, schrieb Linus Torvalds: > On Tue, Nov 13, 2012 at 11:40 AM, Linus Torvalds > wrote: >> >> I have to wonder why you care? As far as I'm concerned, the only valid >> space is space, TAB and CR/LF. >> >> Anything else is *noise*, not space. What's the reason for even caring? > > Btw, expanding the whitespace selection may actually be very > counter-productive. It is used primarily for things like removing > extraneous space at the end of lines etc, and for that, the current > selection of SPACE, TAB and LF/CR is the right thing to do. > > Adding things like FF etc - that are *technically* whitespace, but > aren't the normal kind of silent whitespace - is potentially going to > change things too much. People might *want* a form-feed in their > messages, for all we know. The patch was motivated by the integration of the wildmatch library, which exposes named character classes to users. It replaces a call of fnmatch in match_pathname. Users probably expect [:space:] to mean the same in git as in other programs. I never saw a vertical tab and I can't imagine what it's used for. I'd expect form-feeds to be matched as space, though. Didn't see them very often, admittedly. Nevertheless, it's unfortunate that we have an isspace() that *almost* does what the widely known thing of the same name does. I'd shy away from changing git's version directly, because it's used more than a hundred times in the code, and estimating the impact of adding \v and \f to it. Perhaps renaming it to isgitspace() is a good first step, followed by adding a "standard" version of isspace() for wildmatch? René