212325 - Provide inline EqualsIgnoreCase and CompareIgnoreCase

Status: RESOLVED WORKSFORME

(Core :: XPCOM, enhancement)

Description

[Darin Fisher]

I really think we should provide inline wrappers around Equals(...
nsCaseInsensitive?StringComparator) and Compare to ease coding.  I can't stand
having to type so many characters to get case insensitive comparsion, and I'd
imagine others would agree with me.  Anyways, it is really easy to provide
inline wrappers, that don't in any way break the rule that xpcom shouldn't have
to depend on unicharutils, so why not?  I'll attach a concept patch shortly...

Comment 1

[Darin Fisher]

Attachment: v0 concept patch

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Did you try compiling this in a gcc debug build?

Comment 3

[Darin Fisher]

does gcc 3.1 20020420 (osx 10.2) count? ;)

what are your concerns about gcc?

Comment 4

[jag (Peter Annema)]

Uhm... nsAString is frozen, so you can't add the member functions. And won't
lack of nsAString::EqualsIgnoreCase() when unicharutils isn't linked in give you
build errors?

Comment 5

[Darin Fisher]

>Uhm... nsAString is frozen, so you can't add the member functions. And won't
>lack of nsAString::EqualsIgnoreCase() when unicharutils isn't linked in give you
>build errors?

this doesn't break binary compatibility in any way shape or form.  the fact that
the interface is frozen doesn't prevent us from adding inline helper functions.

you'd only get linker errors if you tried to use nsAString::EqualsIgnoreCase
without including nsUnicharUtils.h, which is desirable i think.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 127467 [details] [diff] [review]
v0 concept patch

Is this still relevant?  If so, could you provide an up-to-date patch?

Comment 7

[Darin Fisher]

Comment on attachment 127467 [details] [diff] [review]
v0 concept patch

yeah, i think this patch is still relevant.  i'll update it.

Comment 8

[Johnny Stenback (:jst)]

Hmm, I guestion the need for these to be inline... Having those methods is nice,
but do they need to be inlined, really?

Comment 9

[Darin Fisher]

good point.  they don't need to be inlined.  since callers have to link to
libunicharutil_s.a anyways, we could just implement nsAString::EqualsIgnoreCase
in there.

Comment 10

[Atul Aggarwal]

I see some EqualsIgnoreCase functions in some string classes
(1. http://mxr.mozilla.org/mozilla-central/source/xpcom/string/src/nsStringObsolete.cpp#985 
2. http://mxr.mozilla.org/mozilla-central/source/xpcom/string/public/nsTString.h#258)

Moreover I see nsCaseInsensitiveCStringComparator class which has case insensitive comparator.
http://mxr.mozilla.org/mozilla-central/source/xpcom/string/public/nsAString.h#77

Do we still needs this for some other string classes like nsTSubString? If no, then we should close this bug as Resolved/WorksForMe.

Comment 11

[Benjamin Smedberg]

Yeah, we fixed this a long time ago.