Bug 27305

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

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.

(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.

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.

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?

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

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

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.

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?

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.)

Created attachment 34225 [details] patch without ICU data files, and updated copyright info forgot to remove the ICU copyright notice

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.

Created attachment 34277 [details] same patch with style fixes Oops, was using an old version of check-webkit-style

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.

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.