Hi Jim,
the tokenizer changes looks good.

I was very confused by the dance between:

// Indicate that the final string should have escapes translated.
                     shouldTranslateEscapes = true;

This:

// Conditionally translate or validate escape sequence and add result to string buffer.
                scanLitChar(pos, !shouldTranslateEscapes, openCount != 1);


And then this:

    private void scanLitChar(int pos, boolean translate, boolean multiline) {


As discussed offline, I think the confusion arises that, in the tokenizer, the 'shouldTranslateEscape' is some kind of a global variable which means "should we translate escapes AFTER reading the string literal", whereas the 'translate' parameter in scanLitChar means "should we translate escapes NOW".

The overlapping (but not identical) meaning of the two is confusing. I also wonder, if one is always the negation of the other, can we refer to the global 'shouldTranslateEscapes' from inside scanLitChar - maybe that will help getting rid of some of the indirection.

Thanks
Maurizio


On 08/11/2019 13:15, Jim Laskey wrote:
Updated

webrev: http://cr.openjdk.java.net/~jlaskey/8233116/webrev.01/index.html 
<http://cr.openjdk.java.net/~jlaskey/8233116/webrev.01/index.html>


On Nov 7, 2019, at 5:23 PM, Brent Christian <brent.christ...@oracle.com> wrote:

Should the new escapes be added to the table in the String.translateEscapes() 
JavaDoc?

-Brent

On 11/7/19 6:22 AM, Jim Laskey wrote:
Please review the following code changes. Provides for the introduction of two new escape 
sequences \<line-terminator> and \s.  \<line-terminator> allows developers to 
express unwieldy string literals in a text block as a cluster of short single line 
segments. The second is to allow developers to express ASCII space, much like \t for tab. 
The changes are primarily in the compiler, with some small changes in 
String::translateEscapes. Small changeset overall.
Thank you.
-- Jim
webrev: http://cr.openjdk.java.net/~jlaskey/8233116/webrev/index.html 
<http://cr.openjdk.java.net/~jlaskey/8233116/webrev/index.html>
jbs: https://bugs.openjdk.java.net/browse/JDK-8233116 
<https://bugs.openjdk.java.net/browse/JDK-8233116>
csr: https://bugs.openjdk.java.net/browse/JDK-8233117 
<https://bugs.openjdk.java.net/browse/JDK-8233117>

Reply via email to