On Sun, 2013-05-19 at 18:24 +0200, Thomas Arnhold wrote:
> Hi,
>
> A quick grep on the source reveals that there case-sensitive and
> case-insensitive comparisons for the font family name. This looks pretty
> suspicious.
>
> git grep -i \"starsymbol\" -- *.cxx
>
> Should we always check case-insensitive or convert all lower-case ones
> to "StarSymbol" (most/all? definitions of this string are mixed case).
We should probably always compare case-insensitivity, it can't really
hurt.
>
> The same holds for "opensymbol". Maybe more...
Nearly every (all?) place which tests for one or does something
dependent on one name will want to do the same thing for the other name.
So, a funny, I said to myself, "we should have a single thing like
IsStarSymbol and use it everywhere", did an opengrok for that, and we
already have *two* of those :-) one in vcl and one in writer. Looking at
the writer one I was reminded that in some places you can have
FONT;FALLBACK;OTHERFALLBACK as the fontname. I'm not sure if the other
various odd looking SearchAscii("StarSymbol") and startsWith tests are
because of that or because of some dev confusion. But a single
isStarSymbol based on the token splitting and case-insensitive compare
in writerhelper.cxx is likely safe everywhere.
> And maybe there is more work with GetFamilyName()..., see
> 79e5615fa103a52ce41ed682b624c13fd9a9d1eb.
Well, its kind of weird to have a test to see if the names are the same,
before going on to see if their foldedcase versions are the same too.
if( rFontName == GetFamilyName() ||
rFontName.equalsIgnoreAsciiCase( GetFamilyName() ) )
the OUString::equalsIgnoreAsciiCase does a (if both-share-the-ptr) opt
so I suggest you make those just into
if (rFontName.equalsIgnoreAsciiCase(GetFamilyName())) ?
C.
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
