On Mon, 12 Jul 2021 16:52:31 GMT, Hannes Wallnöfer <hann...@openjdk.org> wrote:

>> This change adds support for generating HTML links to the type arguments of 
>> enclosing classes when creating a link to an inner class. Previously, only a 
>> link to the inner class was created and the type arguments were even omitted 
>> from the link label.
>> 
>> The new feature to create separate links to the enclosing class and its type 
>> arguments is only activated if the enclosing class has type arguments. If 
>> the enclosing class is non-generic, the old behavior is preserved to create 
>> a single link to the inner class. The reason for this is that a dedicated 
>> link to the enclosing class itself provides little benefit, since it can be 
>> easily reached via the "Enclosing class" link of the inner class. Also, 
>> linking the enclosing type in absence of type arguments makes it hard to see 
>> that there are two links and easy to click on the wrong link by mistake.
>> 
>> On the other hand, for type arguments a separate link should be useful since 
>> it is often not a "nearby" type. It is also easier to detect the different 
>> links than for non-generic nested classes. I came to like this "mixed" 
>> solution best after trying several other approaches.
>
> Hannes Wallnöfer has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   JDK-8259499: Remove extraneous import

See preceding comments.

TL;DR: OK

I found this a hard one to review, to understand the implications. Even reading 
the tests, and the output generated from the test was hard. Eventually, I wrote 
my own simple demo program, with a couple of classes, one generic, one not, 
both with inner classes. Then, one has methods to referring to each of the 
inner classes. This setup allowed me to see the different behavior of 
"side-by-side" links to inner classes with and without a generic outer class.

The observed behavior seems reasonable/sensible.  When the inner class has a 
non-generic outer class, you get a single link for the pair.  When the inner 
class has a generic outer class, the single link is split apart, to give links 
for the outer class and its type args.   That being said, it's not a slam-dunk 
win, it would be almost equally reasonable to use the same linking strategy for 
both generic and non-generic outer classes. I'm OK with the current, let's see 
if we get comments down the road.

The test was mildly confusing for having a series of `{@link}` tags with no 
non-white text to separate them.

Related but separate: The combination of `{@linkplain}` with no description, 
causing the default label to be generated in serif font seems questionable, so 
much so that I almost posted screenshots and comments on the bug.  Maybe this 
should be a warning in doclint. Generally, displaying a code signature in serif 
font seems bad.

Related by separate: The "enclosing class" in the class signature is shown in 
serif font.

-------------

Marked as reviewed by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk17/pull/195

Reply via email to