If we decide we need line and sentence break types we can add them later, but
for now I think we should skip them (not terribly useful for our use case).


http://codereview.chromium.org/6598014/diff/1/src/extensions/experimental/break-iterator.cc
File src/extensions/experimental/break-iterator.cc (right):

http://codereview.chromium.org/6598014/diff/1/src/extensions/experimental/break-iterator.cc#newcode63
src/extensions/experimental/break-iterator.cc:63:
text_value->Utf8Length());
On 2011/02/26 00:31:38, Jungshik Shin wrote:
Chrome's ICU didn't define a macro (U_CHARSET_IS_UTF8) to get ICU to
assume that
|char *| is UTF-8. Perhaps, we'll in the future.

At least for now (and perhaps in the future, too because we don't
control how
other embedders will build ICU),  you should explicitly specify the
encoding for
UnicodeString ctor. You didn't get caught with this because you tested
it on Mac
(or Linux under UTF-8 locale). Under non-UTF-8 locale on Linux or
Windows, your
test would have failed.

You have three alternatives:

1. use UnicodeString:: fromUTF8
2. use a ctor accepting the codepage name : pass "UTF-8"
3. use a ctor accepting UChar* instead of char*.

#1 is faster than #2.

Done.

http://codereview.chromium.org/6598014/diff/1/src/extensions/experimental/break-iterator.cc#newcode154
src/extensions/experimental/break-iterator.cc:154:
raw_template->Set(v8::String::New("assign"),
On 2011/02/26 00:31:38, Jungshik Shin wrote:
How about 'adopt' or 'adoptText'? 'assign' is kinda misleading
(assigning
'string' to 'break iterator' ?)

Done.

http://codereview.chromium.org/6598014/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to