[haiku-gsoc] Re: Localekit patch: various fixes and cleanups
- From: PulkoMandy <pulkomandy@xxxxxxxxx>
- To: haiku-gsoc@xxxxxxxxxxxxx
- Date: Thu, 16 Jul 2009 21:10:23 +0200
> I've looked at the patch and have found the following issues/comments:
>
> - There are now several warnings during the build, some because there are
> still places (e.g. DefaultCatalog.cpp) where fingerprint is used as an int32
> instead of uint32. Some other warnings (during generation of the build tools)
> complain about deprecated headers being used. There should really be no
> warnings during the build of our own sources (maybe except for some
> unavoidable cases).
I did not notice the int32 problems. I'll check again.
About the deprecated header, it is the hash_map that is used
everywhere. Unfortunately, the standardized equivalent will only be
available as part of C++0x, the next C++ standard, and it will ahve a
slightly different api and name. I guess gcc2 will never get updated
for it, and I don't think gcc4 provide a version of it that is not
deprecated. If someone knows of one, tell me.
>
> - The resulting catalogs on the image have none of the locale-attributes
> set (IIRC because of limitations in libroot_build.so). While that can be
> fixed later, I expected that once I started the Locale preflet (which opens
> french.catalog), the attributes of french.catalog would all be set. Language
> and signature are alright now, but the fingerprint is still empty.
The update_attributes function is disabled in the buildtool flavour of
the catalogs. I think I did tht because libroot_build was not able to
do it.
I'll check what the problem is with the fingerprint.
>
> - The locale kit tests no longer build, due to the fingerprint related
> changes. I assume that you haven't noticed because you do not include them in
> the build. Please change your UserBuildConfig such that these tests are
> copied onto the image, which will trigger them being built, too.
woops. will do that...
>
> - The new method 'void escapeQuotedString(BString stringToEscape)' in
> PlainTextCatalog.cpp is a no-op, because it uses a by-value parameter. It
> should be escapeQuotedString(BString& stringToEscape). Additionally, I'd
> suggest using the BString-method CharacterEscape() instead of the three
> invocations of ReplaceAll(). Strangely enough, the resulting english.catkeys
> file still contains the characters in escaped form (and continues to do so
> after the invocations to escapeQuotedString() has been removed). Something is
> fishy, there ;-)
mh, i accidentally removed the & from parseQuotedChars, so they were
never removed. woops again.
>
> - Style hint: in PlainTextCatalog::WriteToFile(), the code fragment where
> textContent is filled should be wrapped such that the '<<' operator is at the
> beginning of a line, not at the end. I have just checked and this does not
> seem to be mentioned in our styleguide, but nevertheless, I believe that's
> how it is done in most of our code.
>
ok, this one is easy to fix.
> cheers,
> Oliver
>
>
So, i'll work on all that.
What can we do about the deprecated hash_map header ?
--
Adrien Destugues / PulkoMandy
Elève ingénieur ENSSAT EII1- www.enssat.fr
GSoC student for Haiku - http://haiku-os.org
GrafX2 project team - http://code.google.com/p/grafx2
Other related posts:
