On Wed, 24 Jan 2024 12:17:19 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
>> Jonathan Gibbons has updated the pull request with a new target base due to >> a merge or a rebase. The pull request now contains eight commits: >> >> - Merge with upstream/master >> - Merge with upstream/master >> - Merge remote-tracking branch 'upstream/master' into >> 8298405.doclet-markdown-v3 >> - Address review comments >> - Fix whitespace >> - Improve handling of embedded inline taglets >> - Customize support for Markdown headings >> - JDK-8298405: Support Markdown in Documentation Comments > > src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DocTreeMaker.java > line 238: > >> 236: && postamble.isEmpty() >> 237: && fullBody.stream().anyMatch(t -> t.getKind() >> == Kind.MARKDOWN) >> 238: ? CommentStyle.JAVADOC_LINE : >> CommentStyle.JAVADOC_BLOCK; > > While clever, it seems to be prone to false positive `JAVADOC_LINE`. Also, it > is inconsistent with `null` and `Position.NOPOS` returned from the `getText` > and `getSourcePos(int)` methods respectively. I'm not worried about false positive `JAVADOC_LINE` than maybe false _negative_ `JAVADOC_LINE`. Generally, the underlying issue here is how to handle weird combinations of `DocTrees` constructed by a user of the public API. For example, should we check, and reject, a `fullBody` containing `MARKDOWN` nodes and non-empty `preamble` or `postamble`, since that combination will never come from `DocCommentParser`. I'm not worried about comparison with `getText` and `getSourcePos`, since there really is no other value for the source text or source position that we can return. But we can infer a best guess for the style. Arguably, we should check the `tags` as well. ------ I dug deeper. We do create synthetic trees in the standard doclet, such as for the descriptions of mandated methods (like `values` and `valueOf` for any enum class. Those synthetic trees do utilize this code path, although the `getStyle` method is not currently invoked. (Verified by changing code to throw `UnsupportedOperationException` and run all the tests.) It's also true those synthetic trees do not leverage any Markdown support at this time. I prefer to leave the code as-is at this time. > src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DocTreeMaker.java > line 704: > >> 702: } >> 703: >> 704: // If the break is well within the span of the string i.e. >> not > > Oh irony! Sentence segmentation in javadoc has some problems with > abbreviations like that. This was fixed 10/26/23. Both forms of the sentence breaker are supposed to get this correct, but yes, chuckle, irony. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1472082408 PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1472089239
