On Fri, 30 Apr 2021 14:06:13 GMT, Hannes Wallnöfer <hann...@openjdk.org> wrote:
> This implements most suggestions implemented in the JBS issue. The heading > for class entries in the page is shortened to "<element-type-kind> <name>", > followed by a simplified signature line like "class X extends Y implements > Serializable". > > Furthermore, package names in package headings now link to the package page, > `@param` info is added to the methods, Externalizable is used for classes > that implement it. I also added a check for content in `@serial` tags to > avoid spurious empty ` ` lines in the output. > > I increased the font size for headings `h4` to `h6` in the stylesheets > because their size was smaller than the default text size, which looked > strange. Although the change in font size from `h3` to `h4` and beyond is now > smaller I think it's still recognizable. Headings `h4` to `h5` are used very > little in Javadoc (mostly static doc files and the serialized-form page). I > looked at some of the static files and they looked good to me with the new > larger heading fonts. > > Although I didn't end up using the `Signatures` class to generate the class > signatures (they're just too simple and too specific in what they list), I > left in some cleanup of `Signatures.TypeSignature`, most significantly moving > the processing of modifiers from `ClassWriterImpl` to > `Signatures.TypeSignature`. > > I only removed the obsolete resources from the English resource file out of > habit of not touching the Chinese and Japanese ones. I guess I could remove > it there too. Generally: wow, very nice cleanup! That being said, there is a potential conflict/race with a separate review to make sealed cases permanent (not preview.) So, this is approved, with a warning to coordinate with @vicente-romero-oracle about the sealed classes review https://github.com/openjdk/jdk/pull/3613 src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Signatures.java line 101: > 99: > 100: private static final Set<String> previewModifiers > 101: = Set.of("sealed", "non-sealed"); This needs to be coordinated with the work to make sealed final. In the changes for that, this set is now empty but the code to process the set (lines 110-126) remains, ready for any future preview modifiers. Also, given this code exists elsewhere, we should probably try and pull it in to a shared method. That being said, Signatures is probably conceptually the right place for the shared method (not Utils.) Also, whether or not we make a shared method, we should probably add an explanatory comment explaining why we keep an empty set around, just in case. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java line 496: > 494: > 495: @SuppressWarnings("preview") > 496: public String modifiersToString(Element e) { Thinking aloud, would this method be better in Signatures? Maybe later... ------------- Marked as reviewed by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3817
