I'll make code changes and get back to you on the rest...
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#newcode81 src/extensions/experimental/break-iterator.cc:81: return v8::Int32::New(break_iterator->next()); UBRK_DONE is actually int32_t and is represented as -1, so this works as intended, and ICU is probably not going to change public API any time soon. I would leave it as is. On 2011/02/26 00:31:38, Jungshik Shin wrote:
When there's no more item, ICU's next() will returb UBRK_DONE (or sth
like
that). How would you signal that to your API users? If you want to use
'-1' for
that, perhaps you need to check the return value from bi->next() and
return '-1'
explicitly for UBRK_DONE.
http://codereview.chromium.org/6598014/diff/1/src/extensions/experimental/break-iterator.cc#newcode110 src/extensions/experimental/break-iterator.cc:110: return v8::Int32::New(UBRK_WORD_IDEO); We can take them out at any time - it's still experimental. On 2011/02/26 00:31:38, Jungshik Shin wrote:
We need to give a bit more thought on what types to expose to
Javascript.
BTW, line/setnence breakers can also have rule_status (
http://icu-project.org/apiref/icu4c/ubrk_8h.html#ad03d8e27f121bcf11eaed0a288786a71
) although they may not be very useful.
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"), I'll fix that. On 2011/02/26 00:31:38, Jungshik Shin wrote:
How about 'adopt' or 'adoptText'? 'assign' is kinda misleading
(assigning
'string' to 'break iterator' ?)
http://codereview.chromium.org/6598014/diff/1/src/extensions/experimental/break-iterator.cc#newcode163 src/extensions/experimental/break-iterator.cc:163: v8::FunctionTemplate::New(BreakIteratorBreakType)); JavaScript has two (actually 3, but one is deprecated) methods to slice a string: slice(from, to) and substring(from, to). So simple prev_pos and pos with slice/substring give you that new method. I think we should keep the API minimal, and I don't think it's going to put any burden on the users given the existing JS apis. On 2011/02/26 00:31:38, Jungshik Shin wrote:
One method is missing; a method that returns the word at the current
position.
A customer of this API can certainly find that out, but wouldn't it
be nice to
offer ourselves?
Yeah, we discussed this and I said that next() is sufficient without
nextWord().
Alternatives:
1. Do nothing. Customers can figure that out 2. Add currentWord() method (either in C++ or JavaScript). 3. Add nextWord() (and remove next())
What do you say?
http://codereview.chromium.org/6598014/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
