On Tue, 25 Mar 2025 04:35:19 GMT, Hannes Wallnöfer <hann...@openjdk.org> wrote:
>> Please review an enhancement to allow switching between the traditional >> condensed footnote-style summary and the summary table format for inherited >> members (types, fields, methods and properties) that are inherited from >> included types. You can test the feature in [the doc bundle I >> uploaded][apidocs] or watch the short screencast below. >> >> [apidocs]: >> https://cr.openjdk.org/~hannesw/8350920/api.00/java.base/module-summary.html >> >> https://github.com/user-attachments/assets/0aaa1f8b-c18b-4922-b704-2b2cdc05ca79 >> >> I added two new protected methods to the `AbstractMemberWriter` class, >> `createInheritedSummaryTable` and `getInheritedSummaryId`. Otherwise this is >> mostly reusing existing functionality (we already had the feature to display >> the signature of an inherited method in the context of the current class). >> >> The writers for non-inheritable members such as constructors or enum >> constants were simplified a bit by implementing formerly abstract methods in >> `AbstractMemberWriter` as concrete methods throwing >> `UnsupportedOperationException` so they don't have to be implemented as >> empty methods in these writers. >> >> The UI to switch between member list presentations is implemented in >> `script.js.template`. If no summary list presentation is available (because >> the supertype is not part of the documentation bundle) nothing changes in >> the UI. >> >> I added a `stripTags()` method to `Content` to return the plain text content >> with HTML tags stripped that could be used in a few other places. The patch >> also adds two new vector graphics files called right.svg and down.svg for >> the right and downwards pointing angle. > > Hannes Wallnöfer has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains four commits: > > - Merge branch 'master' into JDK-8350920 > - Merge branch 'master' into JDK-8350920 > - Add svg files > - 8350920: Allow inherited member summaries to be viewed inline Looks good, I think this a great addition to javadoc. I left a small comment regarding `stripTags` but it's not necessary. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractMemberWriter.java line 253: > 251: var table = getSummaryTable(); > 252: for (Element member : members) { > 253: addMemberSummaryTableRow(typeElement, member, table, > pHelper); I was going to suggest verifying that `table` is not null but it seems like it's not needed. The implementation of `getSummaryTable` doesn't return null (if the summary table doesn't exist, it will attempt to create one). src/jdk.javadoc/share/classes/jdk/javadoc/internal/html/RawHtml.java line 162: > 160: @Override > 161: public Content stripTags() { > 162: return Text.of(rawHtmlContent.replaceAll("<[^>]*>", "")); nit: we use this same transformation `"<[^>]*>"` -> `""` in other places (IndexTaglet and SpecTaglet), could we replace those ad-hoc uses with this method? ------------- Marked as reviewed by nbenalla (Committer). PR Review: https://git.openjdk.org/jdk/pull/23967#pullrequestreview-2743625416 PR Review Comment: https://git.openjdk.org/jdk/pull/23967#discussion_r2029168515 PR Review Comment: https://git.openjdk.org/jdk/pull/23967#discussion_r2029145572
