Re: RFR: 8196027: Remove "Prev" and "Next" links from the javadoc navigation

[Kumar Srinivasan]

I looked at the only changes I suggested.
Ok with me.

Thanks
Kumar

Thanks Kumar. I have made the changes. Please review the updated patch at http://cr.openjdk.java.net/~bpatel/8196027/webrev.02/. Please see my response inline.

On 1/28/2018 6:47 AM, Kumar Srinivasan wrote:

Hi Bhavesh,

|| *src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDoclet.java*

Could you please check the following and remove the computations for prev, next

likely the ListIterator could be simplified to a for-loop.
  240     protected void generateClassFiles(SortedSet<TypeElement> arr, 
ClassTree classtree)
  241             throws DocletException {
  242         List<TypeElement> list = new ArrayList<>(arr);
  243         ListIterator<TypeElement> iterator = list.listIterator();
  244         TypeElement klass = null;
  245         while (iterator.hasNext()) {
  246             TypeElement prev = iterator.hasPrevious() ? klass : null;
  247             klass = iterator.next();
  248             TypeElement next = iterator.nextIndex() == list.size()
  249                     ? null : list.get(iterator.nextIndex());

>> I have updated this and changed it to a for loop. In addition to this, I have also updated generateModuleFiles() and generatePackageFiles() to remove the prev and next computations.

*test/langtools/jdk/javadoc/doclet/testNavigation/TestModuleNavigation.java
*
   25  * @test
   26  * @bug 8196027
   27  * @summary test that navigation links
??

>> I have updated the summary.

Kumar

Looks OK to me.

-- Jon


On 01/24/2018 09:23 PM, Bhavesh Patel wrote:

Please see my response inline. I have made the recommended changes and have uploaded the updated webrev at http://cr.openjdk.java.net/~bpatel/8196027/webrev.01/. There are a few new files in this review from which I have deleted the dead code.

On 1/24/2018 4:44 PM, Jonathan Gibbons wrote:

On 01/24/2018 01:25 PM, Bhavesh Patel wrote:

Hi,
      Please review the fix for change in javadoc navigation bar.

JBS: https://bugs.openjdk.java.net/browse/JDK-8196027.

Webrev: http://cr.openjdk.java.net/~bpatel/8196027/webrev.00/

Regards,
Bhavesh.

In SplitIndexWriter, there are still remnants remaining, such as prev/next fields.

See lines 63-73.

In Contents, there are more remnants, such as the set of prev/next labels.

See lines 143-147, 158-162

Yes. Missed those unused variables. I have removed them from the classes you mentioned and others where its no longer used. Also, I have removed unused imports from these files.

In the new test, checkOutput(file, false, ...) tests are notoriously weak and fragile. In general you want to make them as broad as possible. To that end, I would suggest turning most "+" in lines 81-111 into a comma at the end of the previous line, and reduce all the strings down to either "Next"/"Prev" or "Next&nbspWORD"/"Prev WORD".

I have updated the test.

-- Jon