Bug 27305 – [WINCE] add WinCE-specific unicode implementation

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Joe Mason]

For size reasons, we don't want to link to the full ICU on Wince.  Use a stripped down implementation taking some data structures and routines from ICU and calling WinCE API's directly for others.

[Joe Mason]

Created attachment 32792 [details]
patch adding unicode/wince subdir

[Eric Seidel (no email)]

Comment on attachment 32792 [details]
patch adding unicode/wince subdir

This seems like a bad idea.  My first thought would be that TorchMobile should distribute some sort of support library for this sort of thing.

[Yong Li]

(In reply to comment #2)
> (From update of attachment 32792 [details])
> This seems like a bad idea.  My first thought would be that TorchMobile should
> distribute some sort of support library for this sort of thing.

Why? Other platforms also use either ICU or QT or pango. UnicodeGlib also contains macros from ICU. What we need additionally is just unicode property tables. It seems redundant to create those tables from scratch.

[George Staikos]

I think the main point is that we don't want to require people to use too much outside of the webkit svn repo.  It's quite possible that what we've done for unicode can be reused by other ports later also.

[Joe Mason]

Eric: what specifically about this design do you not like?  Is it the three generated files pulled from ICU?

I will be making a pass for style corrections - what else needs to change to get this in?

[Joe Mason]

Created attachment 34013 [details]
unicode/wince subdir with style fixes, minus generated files

I ran the patch through check-webkit-style and fixed some variable names, and split out the generated files into a separate patch so it's easier to review

[Joe Mason]

Created attachment 34014 [details]
generated files for unicode/wince subdir

[Eric Seidel (no email)]

Comment on attachment 34014 [details]
generated files for unicode/wince subdir

I don't think we should check in these generated files.  I think the wince port should use a support library instead to follow the model of the other ports in the tree.  Alternatively, we could find a way to generate these files at compile time.

[Eric Seidel (no email)]

Comment on attachment 34013 [details]
unicode/wince subdir with style fixes, minus generated files

The project isn't KJS anymore. :)

KJS_UNICODE_WINCE_H

Do we know the ICU license to be compatible with WebCore?  Why is it OK to re-license ICU code under LGPL as you seem to have done?

[Joe Mason]

Created attachment 34223 [details]
patch without ICU data files

Here it is again, with the ICU datafiles and the support code that uses them moved to a support library.  (They are now part of libce at git://code.staikos.net/srv/git/WebKit-CE/libce, which this patch depends on.)

[Joe Mason]

Created attachment 34225 [details]
patch without ICU data files, and updated copyright info

forgot to remove the ICU copyright notice

[Eric Seidel (no email)]

Comment on attachment 34225 [details]
patch without ICU data files, and updated copyright info

Style:
1     const UChar *sourceIterator = source;
 82     const UChar *sourceEnd = source + sourceLength;
 83     UChar *resultIterator = result;
 84     UChar *resultEnd = result + resultLength;

Argument names which don't provide clarity should be removed from headers per our style:
 160         bool isSpace(wchar_t c);
 161         bool isLetter(wchar_t c);
 162         bool isPrintableChar(wchar_t c);
 163         bool isUpper(wchar_t c);
 164         bool isLower(wchar_t c);
 165         bool isPunct(wchar_t c);
 166         bool isDigit(wchar_t c);

I assume these are already covered by other tests?

Thank you very much for posting this much smaller patch.

Unless george is volunteering to fix these when landing, I'm going to r- this and ask you to post a copy with fixed style since you don't have commit-bit yet (to my knowledge).

You will find the "check-webkit-style" script helpful in validating that the style is correct in these files.

[Joe Mason]

Created attachment 34277 [details]
same patch with style fixes

Oops, was using an old version of check-webkit-style

[Eric Seidel (no email)]

Comment on attachment 34277 [details]
same patch with style fixes

Looks OK.

The copy/paste code for toUpper/toLower is bad.  But I'm not gonna fight you about it.

[Adam Barth]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	JavaScriptCore/ChangeLog
	M	JavaScriptCore/wtf/Platform.h
	M	JavaScriptCore/wtf/unicode/Unicode.h
	A	JavaScriptCore/wtf/unicode/wince/UnicodeWince.cpp
	A	JavaScriptCore/wtf/unicode/wince/UnicodeWince.h
Committed r46911

Not sure why, but bugzilla-tool had trouble parsing the ChangeLog for this patch.