On Thu, 6 May 2021 14:37:11 GMT, Jonathan Gibbons <j...@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 `&nbsp;` 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

Reply via email to