On Thu, 27 May 2021 03:20:34 GMT, Jonathan Gibbons <[email protected]> wrote:
> Please review an update to `jdk.javadoc/jdk.javadoc.doclets.Reporter` to add > 3 new methods and to improve the descriptions of other parts of the interface. > > The new methods provide access to the underlying streams (informally, for > standard output and diagnostic output), and a new `report` method to report > diagnostics at an arbitrary position in a file being read by a doclet, or a > taglet within a doclet. Thank you so much for doing this, Jon. Although this PR is good by itself, it will be particularly useful for snippets (JEP 413). While I'm still reviewing this PR, you could have a look at the comments I have made so far. As far as I understand https://github.com/openjdk/jdk/pull/4077 will be closed as superseded by this PR? src/jdk.javadoc/share/classes/jdk/javadoc/doclet/Reporter.java line 39: > 37: * Interface for reporting diagnostics and other messages. > 38: * > 39: * <p>Diagnostics consist of a diagnostic {@link Diagnostic.Kind kind} > and a message, Consider moving "diagnostic" into the `@link` label: `{@link Diagnostic.Kind diagnostic kind}`. src/jdk.javadoc/share/classes/jdk/javadoc/doclet/Reporter.java line 69: > 67: * > 68: * @param kind the kind of diagnostic > 69: * @param message message to print Prepend "message to print" with "the". Alternatively, consider using "the message to be printed", which seems typical in this file. src/jdk.javadoc/share/classes/jdk/javadoc/doclet/Reporter.java line 98: > 96: * @param file the file > 97: * @param start the beginning of the enclosing range > 98: * @param pos the position Unless there's a reason not to, I would prefer to have an inequality clearly showing how these 3 indexes relate to each other. For example, is it the case that `{@code start <= pos <= end}`? src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/Messages.java line 104: > 102: * @param start the start of a range of characters to be associated > with the end > 103: * @param pos the position to be associated with the end > 104: * @param end the end of a range of characters to be associated > with the end "Associated with end"? I believe it should either be "error" or "message", not "end". src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/Messages.java line 106: > 104: * @param end the end of a range of characters to be associated > with the end > 105: * @param key the name of a resource containing the message to be > printed > 106: * @param args optional arguments to be replaced in the message. Remove the trailing period. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/Messages.java line 157: > 155: * @param start the start of a range of characters to be associated > with the end > 156: * @param pos the position to be associated with the end > 157: * @param end the end of a range of characters to be associated > with the end Same here: "associated with the end"? src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/Messages.java line 159: > 157: * @param end the end of a range of characters to be associated > with the end > 158: * @param key the name of a resource containing the message to be > printed > 159: * @param args optional arguments to be replaced in the message. Same here: remove the trailing period. src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line 180: > 178: */ > 179: public Messager(Context context, String programName) { > 180: // use the current values of System.out, System,err, in case > they have been redirected Replace "," with ".": System.err src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line 435: > 433: > 434: /** > 435: * Prints the error and warning counts, if any, to. the diagnostic > writer Move period from after "to" to the end of the sentence. test/langtools/jdk/javadoc/doclet/testReporterStreams/TestReporterStreams.java line 1: > 1: /* Indent. ------------- PR: https://git.openjdk.java.net/jdk/pull/4216
