https://codereview.chromium.org/121173009/diff/1/src/api.cc
File src/api.cc (right):
https://codereview.chromium.org/121173009/diff/1/src/api.cc#newcode4519
src/api.cc:4519: // @TODO use uint16_t for previous?
On 2014/01/04 15:56:45, dcarney wrote:
previous is an int because of some special values used in the encoder,
like
kNoPreviousCharacter
Makes sense. Thx.
https://codereview.chromium.org/121173009/diff/1/src/api.cc#newcode4529
src/api.cc:4529: return written + Utf8::Encode(buffer, code_point,
false);
On 2014/01/04 15:56:45, dcarney wrote:
having the length calculation here is too late. String::WriteUtf8
must compute
the identical length, which it can't do if you're potentially changing
it here.
Ok, I'll take a closer look.
https://codereview.chromium.org/121173009/diff/1/src/unicode-inl.h
File src/unicode-inl.h (right):
https://codereview.chromium.org/121173009/diff/1/src/unicode-inl.h#newcode114
src/unicode-inl.h:114: unsigned Utf8::Encode(char* str, uchar c, bool
allow_invalid) {
On 2014/01/04 15:56:45, dcarney wrote:
the name of the third argument is not the same as the declaration.
Thx, will fix.
https://codereview.chromium.org/121173009/diff/1/src/unicode-inl.h#newcode129
src/unicode-inl.h:129: } else if (c <= kMaxThreeByteChar) {
On 2014/01/04 15:56:45, dcarney wrote:
can't chop this section out - need backwards compatibility for the
other call
sites of Encode
I'd prefer changing the other call sites if needed. I'll look into this
today. But yeah, if that can't be done for some reason, I'll put this
code back.
https://codereview.chromium.org/121173009/diff/1/src/unicode.h
File src/unicode.h (right):
https://codereview.chromium.org/121173009/diff/1/src/unicode.h#newcode53
src/unicode.h:53: const int kReplacementCharacter = 0xFFFD;
On 2014/01/04 15:56:45, dcarney wrote:
should probably be in the Utf8 class
I don't feel strongly about it, but technically it's not directly
related to UTF-8, but rather provided for all unicode encoders/decoders
that might need it.
https://codereview.chromium.org/121173009/diff/1/src/unicode.h#newcode158
src/unicode.h:158: static inline unsigned Encode(char* str, uchar c,
bool replace_surrogates);
On 2014/01/04 15:56:45, dcarney wrote:
this is not backwards compatible.
Is there a good reason to keep compatibility, rather than changing the
call sites?
https://codereview.chromium.org/121173009/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
