Addressed comments.

http://codereview.chromium.org/10857030/diff/2001/include/v8.h
File include/v8.h (right):

http://codereview.chromium.org/10857030/diff/2001/include/v8.h#newcode1075
include/v8.h:1075: UTF8_ENCODING     = 1,
On 2012/08/21 12:21:34, Erik Corry wrote:
The names are not UTF8 or UTF16, but rather UTF-8 and UTF-16.  These
should
therefore be called UTF_8_ENCODING and UTF_16_ENCODING.  Also this
needs fixing
several places in the comments.

Done.

http://codereview.chromium.org/10857030/diff/2001/include/v8.h#newcode1079
include/v8.h:1079: STRICT_ASCII_HINT = 1 << 16,
On 2012/08/21 12:21:34, Erik Corry wrote:
I think STRICT_ASCII_HINT should just be called ASCII_HINT

Done.

http://codereview.chromium.org/10857030/diff/2001/include/v8.h#newcode1100
include/v8.h:1100: // Latin1 characters.
On 2012/08/21 12:21:34, Erik Corry wrote:
This one doesn't support PRESERVE_ASCII_NULL, or rather it is always
on.  That
should be noted.  May also apply to WriteUtf8.

Done.

http://codereview.chromium.org/10857030/diff/2001/include/v8.h#newcode1211
include/v8.h:1211: * strings are converted to ASCII or two-byte string
depending on whether
On 2012/08/21 12:21:34, Erik Corry wrote:
string -> strings,

Done.

http://codereview.chromium.org/10857030/diff/2001/include/v8.h#newcode1232
include/v8.h:1232: * If the string is external, return the its encoding
(Latin1 or UTF16)
On 2012/08/21 12:21:34, Erik Corry wrote:
the its -> its

Done.

http://codereview.chromium.org/10857030/diff/2001/include/v8.h#newcode1248
include/v8.h:1248: * Allocates a new string from either UTF-8-,
Latin1-encoded data.
On 2012/08/21 12:21:34, Erik Corry wrote:
"-," should be "or "

Done.

http://codereview.chromium.org/10857030/diff/2001/include/v8.h#newcode1249
include/v8.h:1249: * The second parameter 'length' gives the buffer
length.  If the data is
On 2012/08/21 12:21:34, Erik Corry wrote:
is UTF-8 encoded -> may contain zero bytes

Done.

http://codereview.chromium.org/10857030/diff/2001/include/v8.h#newcode1254
include/v8.h:1254: * whether the string contains ASCII characters.  In
case of Latin1, the
On 2012/08/21 12:21:34, Erik Corry wrote:
In case of -> In the case of

Done.

http://codereview.chromium.org/10857030/diff/2001/include/v8.h#newcode1259
include/v8.h:1259: int encoding = UTF8_ENCODING);
On 2012/08/21 12:21:34, Erik Corry wrote:
Surely this should be a StringEncoding and not an int.

The encoding here may additionally contain NOT_ASCII_HINT or ASCII_HINT,
therefore I can't simply use an enum, since e.g. (ASCII_HINT |
UTF-8-ENCODING) is not a valid element of the enum.

http://codereview.chromium.org/10857030/diff/2001/include/v8.h#newcode1328
include/v8.h:1328: * If the data contains non-ASCII character, the
string is created as new
On 2012/08/21 12:21:34, Erik Corry wrote:
contains -> contains a
as new -> as a new

Done.

http://codereview.chromium.org/10857030/diff/2001/include/v8.h#newcode1330
include/v8.h:1330: * resource immediately.  This is because V8 is
currently unable to handle
On 2012/08/21 12:21:34, Erik Corry wrote:
is currently unable -> is unable

Done.

http://codereview.chromium.org/10857030/diff/2001/src/heap-inl.h
File src/heap-inl.h (right):

http://codereview.chromium.org/10857030/diff/2001/src/heap-inl.h#newcode91
src/heap-inl.h:91: // Assert that the strict ascii hint is correct.
On 2012/08/21 12:21:34, Erik Corry wrote:
ascii -> ASCII
here and below

Done.

http://codereview.chromium.org/10857030/diff/2001/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/10857030/diff/2001/src/objects.h#newcode7097
src/objects.h:7097: STRICT_ASCII = 1,
On 2012/08/21 12:21:34, Erik Corry wrote:
Again, the STRICT_ seems wrong.

Done.

http://codereview.chromium.org/10857030/diff/2001/src/v8-counters.h
File src/v8-counters.h (right):

http://codereview.chromium.org/10857030/diff/2001/src/v8-counters.h#newcode256
src/v8-counters.h:256: SC(string_length_ascii, V8.StringLengthAScii)
                  \
On 2012/08/21 12:21:34, Erik Corry wrote:
AScii is a strange way to capitalize.  I would accept Ascii or ASCII.

This was a typo.  Done.

http://codereview.chromium.org/10857030/

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

Reply via email to