On Thu, 6 May 2021 14:37:11 GMT, Jonathan Gibbons <[email protected]> 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.
>
> 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.
> 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 #3613
Thanks for the review, Jon. @vicente-romero-oracle please go ahead with your
PR, I'll update mine afterwards. (I should have been aware you were removing
these sealed preview bits.)
-------------
PR: https://git.openjdk.java.net/jdk/pull/3817
