On Tue, 21 Sep 2021 01:33:09 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
>> Pavel Rappo has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 37 commits: >> >> - Merge branch 'master' into 8266666 >> - Improve markup error messages >> >> * Cleans up error string format >> * Internationalizes error messages, mapping them to fewer resource keys >> * Fixes a few bugs related to error messages, including: inaccurate >> caret positioning and runaway resource name (i.e. the resource key wasn't >> translated into an error) >> - Merge branch 'master' into 8266666 >> - Address trivial feedback >> - Address feedback >> >> 1. Reverts changes from CommentUtils.java (unused methods) >> 2. Changes misguided regex-related assertion to tests >> - Remove trailing whitespace to satisfy jcheck >> - Clean up code >> >> Removes stale FIXMEs, TODOs and comments; downgrades FIXMEs to TODOs >> where possible; wraps overly long lines. >> - Merge branch 'master' into 8266666 >> - Merge branch 'master' into 8266666 >> - Refactor SnippetTaglet >> >> Improves code style; reformats. >> - ... and 27 more: >> https://git.openjdk.java.net/jdk/compare/4b3a4fff...edb7cf85 > > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/doclets.properties > line 400: > >> 398: # 0: path; 1: exception >> 399: doclet.error_setting_snippet_path=\ >> 400: Error setting snippet path {0}: {1} > > Add newline at end of file Although it wasn't there to begin with, adding it won't be as big a deal as reformatting code in other places you suggested earlier. > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/snippet/MarkupParser.java > line 220: > >> 218: switch (ch) { >> 219: case ':': // indicates that the instruction relates to the >> next line >> 220: case ' ': case '\t': > > What about other whitespace? Is that already covered? Which other whitespace? If you are talking about `\r` or `\n`, then the string parsed by `MarkupParser` cannot have those. `MarkupParser` gets a single line without a trailing line terminator. > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/snippet/ParseException.java > line 35: > >> 33: * This exception is only used to capture a user-facing error message. >> 34: * The message supplier is accepted not to control when to obtain a >> message, >> 35: * but to abstract how to obtain in. > > typo "in"? maybe "one" or "it"? Good catch! It should've been "it". > test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetTag.java line 88: > >> 86: /* >> 87: * While the "id" and "lang" attributes are advertised in JEP 413, >> they are >> 88: * currently unused by the implementation. The goal of this test is >> to make > > id's should be propagated to the output HTML About empty id's and propagating id's to the output HTML. I think we can address these in a follow-up issue. Would you be okay with it? > test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetTag.java line 473: > >> 471: // out of order with respect to the errors checked above. >> 472: // This is because the below errors are modelled as exceptions >> thrown >> 473: // at parse time, when there are no doc trees yet. And the >> above errors > > This would read better as "the errors below" and "the errors above" > https://www.grammarphobia.com/blog/2012/12/below.html Thanks. ------------- PR: https://git.openjdk.java.net/jdk/pull/4795
