On Mon, 20 Mar 2023 20:31:46 GMT, Jim Laskey <jlas...@openjdk.org> wrote:
>> Enhance the Java programming language with string templates, which are >> similar to string literals but contain embedded expressions. A string >> template is interpreted at run time by replacing each expression with the >> result of evaluating that expression, possibly after further validation and >> transformation. This is a [preview language feature and >> API](http://openjdk.java.net/jeps/12). > > Jim Laskey has updated the pull request incrementally with two additional > commits since the last revision: > > - Tidy javadoc > - Rename StringTemplate classes I like the new naming scheme! src/java.base/share/classes/java/lang/StringTemplate.java line 28: > 26: package java.lang; > 27: > 28: import java.lang.Object; You might want to do another pass to check for unused imports src/java.base/share/classes/java/lang/StringTemplate.java line 44: > 42: * {@link StringTemplate} is the run-time representation of a string > template or > 43: * text block template in a template expression. > 44: * Extra newline? src/java.base/share/classes/java/lang/StringTemplate.java line 56: > 54: * {@link StringTemplate} is primarily used in conjunction with a > template processor > 55: * to produce a string or other meaningful value. Evaluation of a > template expression > 56: * first produces an instance of {@link StringTemplate}, representing the > template The "template of the template expression" - is this nomenclature official (e.g. backed by any text in the JLS?). I must admit I find it a tad jarring. src/java.base/share/classes/java/lang/StringTemplate.java line 69: > 67: * List<Object> values = st.values(); > 68: * } > 69: * The value of {@code fragments} will be equivalent to {@code > List.of("", " + ", " = ", "")}, This is a bit hard to parse, due to the use of `the value of` (which then becomes problematic in the next para, as we are talking about `values`). Either change the name of the variable `values` in the snippet, or use some other locution, like "the list {@code fragments} is equivalent to ..." etc. src/java.base/share/classes/java/lang/StringTemplate.java line 324: > 322: * assert Objects.equals(sc, "abcxyz"); > 323: * } > 324: * the last character {@code "c"} from the first string is > juxtaposed with the first I was playing with this example in jshell: jshell> var st1 = RAW."{1}" st1 ==> StringTemplate{ fragments = [ "", "" ], values = [1] } jshell> var st2 = RAW."{2}" st2 ==> StringTemplate{ fragments = [ "", "" ], values = [2] } jshell> StringTemplate.combine(st1, st2); $7 ==> StringTemplate{ fragments = [ "", "", "" ], values = [1, 2] } The result is obviously well-formed - but I'm not sure I can derive what the method does just by reading the javadoc. The javadoc says: Fragment lists from the StringTemplates are combined end to end with the last fragment from each StringTemplate concatenated with the first fragment of the next I think I see what you want to say - (e.g. the end fragment of string template `N` is concatenated with the starting fragment of string template `N + 1`). src/java.base/share/classes/java/lang/StringTemplate.java line 431: > 429: * Java compilation unit.<p>The result of interpolation is not > interned. > 430: */ > 431: static final StringProcessor STR = StringTemplate::interpolate; `static final` is redundant here (we are in an interface) src/java.base/share/classes/java/lang/StringTemplate.java line 454: > 452: * This interface describes the methods provided by a generalized > string template processor. The > 453: * primary method {@link Processor#process(StringTemplate)} is used > to validate > 454: * and compose a result using a {@link StringTemplate > StringTemplate's} fragments and values lists. nit: `{@link StringTemplate StringTemplate's}` will probably not render as you'd expect (as it will all be preformat, including the `'s`). src/java.base/share/classes/java/lang/runtime/ReferenceKey.java line 47: > 45: */ > 46: @PreviewFeature(feature=PreviewFeature.Feature.STRING_TEMPLATES) > 47: interface ReferenceKey<T> { This (and other) class are package-private. Do we still need @PreviewFeature for stuff like this, since it's not meant to be used publicly? src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransLiterals.java line 226: > 224: List.nil(), expressions); > 225: valuesArray.type = new ArrayType(syms.objectType, > syms.arrayClass); > 226: return bsmCall(names.process, > names.newLargeStringTemplate, syms.stringTemplateType, nit: the indy name is irrelevant here - that said, `process` is a tad confusing, since it's also the name of a StringTemplate method. src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransLiterals.java line 252: > 250: staticArgsTypes = > staticArgsTypes.append(syms.stringType); > 251: } > 252: return bsmCall(names.process, names.processStringTemplate, > tree.type, What happens if we have too many fragments here? (e.g. > 200). That case seems handled in the RAW case. src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransLiterals.java line 281: > 279: make.at(tree.pos); > 280: > 281: if (processor == null || isNamedProcessor(names.RAW)) { is `processor == null` still a thing? ------------- PR Review: https://git.openjdk.org/jdk/pull/10889#pullrequestreview-1350457321 PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1143370713 PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1143371242 PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1143373826 PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1143379210 PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1143441733 PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1143444292 PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1143448931 PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1143484050 PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1143501675 PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1143507003 PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1143487363
