RFR: JDK-8261263: Simplify javadoc link code

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: JDK-8261263: Simplify javadoc link code

Hannes Wallnöfer
This is a cleanup of the javadoc link generating code.

There is a simple "horizontal" component to this change that removes the `strong` parameters from most `HtmlDocletWriter#getDocLink` methods which where the value used was always `false`, and related changes in all the code using these methods.

The slightly more complex part of this change are changes in `LinkInfo[Impl]` and `LinkFactory[Impl]`. Here the target was to reduce the number of booean fields in `LinkInfo` and their interaction with the code, which was quite hard to grasp. I managed to replace several fields controlling generation of type parameter links with a single `includeTypeParameterLinks()` method. The use of this method and the remaining boolean fields in the code is quite straightforward.

I also removed some bits of dead code and simplified the control flow a bit by trying to do things only in one place and one way when possible.

The code passes all javadoc tests and generates documentation identical to the old code.

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

Commit messages:
 - JDK-8261263: Reformat and rephrase comments
 - JDK-8261263: Simplify javadoc link code

Changes: https://git.openjdk.java.net/jdk/pull/2437/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2437&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261263
  Stats: 257 lines in 15 files changed: 41 ins; 153 del; 63 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2437.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2437/head:pull/2437

PR: https://git.openjdk.java.net/jdk/pull/2437
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8261263: Simplify javadoc link code

Jonathan Gibbons-2
On Fri, 5 Feb 2021 20:59:45 GMT, Hannes Wallnöfer <[hidden email]> wrote:

> This is a cleanup of the javadoc link generating code.
>
> There is a simple "horizontal" component to this change that removes the `strong` parameters from most `HtmlDocletWriter#getDocLink` methods which where the value used was always `false`, and related changes in all the code using these methods.
>
> The slightly more complex part of this change are changes in `LinkInfo[Impl]` and `LinkFactory[Impl]`. Here the target was to reduce the number of booean fields in `LinkInfo` and their interaction with the code, which was quite hard to grasp. I managed to replace several fields controlling generation of type parameter links with a single `includeTypeParameterLinks()` method. The use of this method and the remaining boolean fields in the code is quite straightforward.
>
> I also removed some bits of dead code and simplified the control flow a bit by trying to do things only in one place and one way when possible.
>
> The code passes all javadoc tests and generates documentation identical to the old code.

A couple of places where there are remnants of `isStrong`; are they still required?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/links/LinkInfo.java line 75:

> 73:      * True if the link should be strong.
> 74:      */
> 75:     public boolean isStrong = false;

This is pervasively disappearing elsewhere. Is it still needed here?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/links/LinkInfo.java line 149:

> 147:                 ", isVarArg=" + isVarArg +
> 148:                 ", label=" + label +
> 149:                 ", isStrong=" + isStrong +

isStrong again

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

PR: https://git.openjdk.java.net/jdk/pull/2437
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8261263: Simplify javadoc link code

Jonathan Gibbons-2
In reply to this post by Hannes Wallnöfer
On Fri, 5 Feb 2021 20:59:45 GMT, Hannes Wallnöfer <[hidden email]> wrote:

> This is a cleanup of the javadoc link generating code.
>
> There is a simple "horizontal" component to this change that removes the `strong` parameters from most `HtmlDocletWriter#getDocLink` methods which where the value used was always `false`, and related changes in all the code using these methods.
>
> The slightly more complex part of this change are changes in `LinkInfo[Impl]` and `LinkFactory[Impl]`. Here the target was to reduce the number of booean fields in `LinkInfo` and their interaction with the code, which was quite hard to grasp. I managed to replace several fields controlling generation of type parameter links with a single `includeTypeParameterLinks()` method. The use of this method and the remaining boolean fields in the code is quite straightforward.
>
> I also removed some bits of dead code and simplified the control flow a bit by trying to do things only in one place and one way when possible.
>
> The code passes all javadoc tests and generates documentation identical to the old code.

I see that `isStrong` is still used, but eventually maps into a style of `typeNameLink` down in the `Links` class, line 188. I believe the name `strong` refers back to using the HTML `<strong>` element, which no longer exists.

Can we now or soon change the name or at least the doc comments for this parameter into something more meaningful?

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

Marked as reviewed by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2437
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8261263: Simplify javadoc link code

Jonathan Gibbons-2
On Tue, 9 Feb 2021 02:08:45 GMT, Jonathan Gibbons <[hidden email]> wrote:

>> This is a cleanup of the javadoc link generating code.
>>
>> There is a simple "horizontal" component to this change that removes the `strong` parameters from most `HtmlDocletWriter#getDocLink` methods which where the value used was always `false`, and related changes in all the code using these methods.
>>
>> The slightly more complex part of this change are changes in `LinkInfo[Impl]` and `LinkFactory[Impl]`. Here the target was to reduce the number of booean fields in `LinkInfo` and their interaction with the code, which was quite hard to grasp. I managed to replace several fields controlling generation of type parameter links with a single `includeTypeParameterLinks()` method. The use of this method and the remaining boolean fields in the code is quite straightforward.
>>
>> I also removed some bits of dead code and simplified the control flow a bit by trying to do things only in one place and one way when possible.
>>
>> The code passes all javadoc tests and generates documentation identical to the old code.
>
> I see that `isStrong` is still used, but eventually maps into a style of `typeNameLink` down in the `Links` class, line 188. I believe the name `strong` refers back to using the HTML `<strong>` element, which no longer exists.
>
> Can we now or soon change the name or at least the doc comments for this parameter into something more meaningful?

Updated review to Approved, but I hope we can do something now or soon to clean up the remaining use of `isStrong`. Either eliminate it or rename to something more appropriate.

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

PR: https://git.openjdk.java.net/jdk/pull/2437
Reply | Threaded
Open this post in threaded view
|

Integrated: JDK-8261263: Simplify javadoc link code

Hannes Wallnöfer
In reply to this post by Hannes Wallnöfer
On Fri, 5 Feb 2021 20:59:45 GMT, Hannes Wallnöfer <[hidden email]> wrote:

> This is a cleanup of the javadoc link generating code.
>
> There is a simple "horizontal" component to this change that removes the `strong` parameters from most `HtmlDocletWriter#getDocLink` methods which where the value used was always `false`, and related changes in all the code using these methods.
>
> The slightly more complex part of this change are changes in `LinkInfo[Impl]` and `LinkFactory[Impl]`. Here the target was to reduce the number of booean fields in `LinkInfo` and their interaction with the code, which was quite hard to grasp. I managed to replace several fields controlling generation of type parameter links with a single `includeTypeParameterLinks()` method. The use of this method and the remaining boolean fields in the code is quite straightforward.
>
> I also removed some bits of dead code and simplified the control flow a bit by trying to do things only in one place and one way when possible.
>
> The code passes all javadoc tests and generates documentation identical to the old code.

This pull request has now been integrated.

Changeset: b0e7e5ab
Author:    Hannes Wallnöfer <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/b0e7e5ab
Stats:     257 lines in 15 files changed: 41 ins; 153 del; 63 mod

8261263: Simplify javadoc link code

Reviewed-by: jjg

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

PR: https://git.openjdk.java.net/jdk/pull/2437