On Fri, 21 May 2021 17:39:43 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
>> Jonathan Gibbons has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> minor cleanup
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/WorkArounds.java
> line 543:
>
>> 541: * Returns whether or not to permit dynamically loaded components
>> to access
>> 542: * part of the javadoc internal API. The flag is the same (hidden)
>> compiler
>> 543: * option that allows javac plugins and annotation processors to
>> javac internal
>
> Do you use "allow x to y" here in the sense of "letting x into y"? If so,
> consider using "allow x into y" instead of "allow x to y". Otherwise it might
> sound like there's a verb missing: allow to (access? enter?) javac internal
> API. My non-native English speaker's two cents.
Yes, there was a missing verb. Fixed.
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line 67:
>
>> 65: * of diagnostic messages and to avoid code duplication.
>> 66: *
>> 67: * The class is a subtype of javac's Log, and is primarily a transducer
>> between
>
> Would "adapter" be more suitable than "transducer"?
Both are valid, but agreed that "adapter" is a more common term.
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line 92:
>
>> 90: * <li>{@code Diagnostic.Kind} -- maps to {@code DiagnosticType} and
>> {@code Set<DiagnosticFlag>}
>> 91: * <li>{@code Element} -- maps to {@code DiagnosticSource and {@code
>> DiagnosticPosition}
>> 92: * <li>{@code DocTreePath} -- maps to {@code DiagnosticSource and
>> {@code DiagnosticPosition}
>
> Close `@code` tags.
oops
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line 96:
>
>> 94: *
>> 95: * The javac layer deals primarily in pre-localized (key, args) pairs,
>> 96: * while the javadoc layer, especially the {@code Reporter} interface,
>> deals in localized strings.
>
> If it is not a typo, consider changing "deal in" to something simpler.
I'll try. Maybe something based on "operates on" or "uses" or something.
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line
> 125:
>
>> 123: private ToolEnvironment toolEnv;
>> 124:
>> 125: /** The utility class to access the positions of items in doc
>> comments., */
>
> Remove that trailing comma.
oops
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line
> 332:
>
>> 330:
>> 331: /**
>> 332: * Print a "notice" message.
>
> Add "s" to "print" for consistency with doc comment of other methods in this
> class.
Definitely.
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line
> 448:
>
>> 446: * such as "Generating class ...". Therefore, for now, we detect
>> and report those
>> 447: * messages directly. (A better solution would be to expose access
>> to the output
>> 448: * and error streams via {@code DocletEnvironment}.
>
> Sentence: either remove "(" or add ")".
Add ")" and change DocletEnvironment to Reporter for now, and to delete the
~sentence~ paragraph when we add streams to Reporter.
> test/langtools/jdk/javadoc/doclet/testDiagsLineCaret/MyTaglet.java line 127:
>
>> 125: @Override
>> 126: public Void visitEntity(EntityTree node, Diagnostic.Kind k)
>> {
>> 127: if (node.getName().contentEquals("#x1f955")) {
>
> Is this... a carrot (🥕)?
https://www.fileformat.info/info/unicode/char/search.htm :-)
> test/langtools/jdk/javadoc/doclet/testMissingComment/TestMissingComment.java
> line 115:
>
>> 113: javadoc("-d", base.resolve("api").toString(),
>> 114: "-Xdoclint:missing",
>> 115: "--no-platform-links",
>
> More of those... sigh.
A different solution would be to disable platform links by default (in
JavadocTester) and only enables for specific tests. But that would be a minor
paradigm shift of JavadocTester providing default options.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4074
