RFR : JDK-8215599 : Remove support for javadoc "frames" mode

[Jonathan Gibbons]

OK.


Follow up discussion ...

"grep" says that now that the frame files are going away, 
AbstractPackageIndexWriter only has a single subtype, 
PackageIndexWriter. Likewise AbstractModuleIndexWriter is only subtyped 
by ModuleIndexWriter.  In both cases, it might be reasonable to coalesce 
the Abstract* type and the subtype, thus eliminating the need for the 
Abstract* type.  You should be able to do this with an IDE, although you 
might need to carefully examine the results, especially with regard to 
the ordering that might be chosen by the IDE.

-- Jon

On 03/29/2019 01:52 AM, Priya Lakshmi Muthuswamy wrote:
>
> Hi Jon,
>
> Thanks for the review.
>
> In AbstractModuleIndexWriter.java, removed the empty list for 
> addModulePackagesIndexContents method but missed the addIndexContents.
> Updated the webrev with the suggestions.
>
> updated webrev : 
> http://cr.openjdk.java.net/~pmuthuswamy/8215599/webrev.02/ 
> <http://cr.openjdk.java.net/%7Epmuthuswamy/8215599/webrev.02/>
>
> Thanks,
> Priya
>
> On 3/29/2019 3:09 AM, Jonathan Gibbons wrote:
>>
>> Priya,
>>
>> Nearly there: minor comments below:
>>
>> In this file:
>>
>> http://cr.openjdk.java.net/%7Epmuthuswamy/8215599/webrev.01/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractModuleIndexWriter.java.sdiff.html
>>
>> you have not removed the empty list that I reported in the previous 
>> review.   Look at lines 223-224 in the new code.
>>
>> http://cr.openjdk.java.net/%7Epmuthuswamy/8215599/webrev.01/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractPackageIndexWriter.java.sdiff.html
>>
>> line 112, You've deleted the @param documentation, but not the actual 
>> parameter in the signature!
>>
>> General comment: IndexRedirectWriter: this class needs a better 
>> explanatory comment. I figured out why we still need it, but it took 
>> a while. I'll file a separate bug to improve the comment.
>>
>> ModuleIndexWriter, PackageIndexWriter
>> I think the main comment may be wrong ... the filename is always 
>> "index.html", isn't it, not "overview-summary.html"
>>
>> test/langtools/jdk/javadoc/doclet/WindowTitles/WindowTitles.java.
>> line 57 .... previously, the arguments were vertically aligned. The 
>> edit broke the alignment.
>>
>> -- Jon
>>
>>
>> On 03/15/2019 02:25 AM, Priya Lakshmi Muthuswamy wrote:
>>>
>>> Hi Jon,
>>>
>>> I have updated the webrev.
>>> http://cr.openjdk.java.net/~pmuthuswamy/8215599/webrev.01/
>>>
>>> changeset id of the tip - 54142:6ab293f66cae
>>>
>>> Thanks,
>>> Priya
>>>
>>> On 3/15/2019 4:33 AM, Jonathan Gibbons wrote:
>>>>
>>>> Priya,
>>>>
>>>> I'm trying to look at these changes with an IDE, which requires a 
>>>> repo.  The patch in the webrev does not apply cleanly to the 
>>>> current tip of the jdk/jdk repo.
>>>>
>>>> Can you either post an updated webrev, or just post the changeset 
>>>> id of the rip of your repo, so that I can "hg update" my repo so 
>>>> that the patch will apply cleanly.
>>>>
>>>> -- Jon
>>>>
>>>>
>>>> On 03/11/2019 06:28 AM, Priya Lakshmi Muthuswamy wrote:
>>>>>
>>>>> Hi Jon,
>>>>>
>>>>> Thanks for the review.
>>>>> I have updated the webrev.
>>>>>
>>>>> webrev : http://cr.openjdk.java.net/~pmuthuswamy/8215599/webrev.01/
>>>>>
>>>>> Reg ModuleIndexWriter, old lines 98-106:
>>>>> After removing frames, the methods AllClassesLink/AllModulesLink 
>>>>> have been removed as they implemented in frame classes.
>>>>> With out these, we are just creating empty list. That's the reason 
>>>>> for deleting these lines.
>>>>> This change is similar to AbstractModuleIndexWriter lines old 
>>>>> lines 251-258, in the updated webrev, i have removed the creation 
>>>>> of empty list.
>>>>>
>>>>> Thanks,
>>>>> Priya
>>>>>
>>>>> On 3/9/2019 6:48 AM, Jonathan Gibbons wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 03/08/2019 03:12 AM, Priya Lakshmi Muthuswamy wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Kindly review the changes for removal of frames.
>>>>>>> Removed the script for setting window title which was needed for 
>>>>>>> frames and also the boolean includeScript,
>>>>>>> which was false for overview-frame,allclasses-frame and 
>>>>>>> package-frame and true for all other cases.
>>>>>>>
>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8215599
>>>>>>> webrev: http://cr.openjdk.java.net/~pmuthuswamy/8215599/webrev.00/
>>>>>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8215603
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Priya
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> AbstractModuleIndexWriter.java
>>>>>> lines 243,244
>>>>>> You're creating an empty list and adding it to the header.
>>>>>> I think you can delete these lines.
>>>>>>
>>>>>> AbstractPackageIndexWriter.java
>>>>>> line 43:  bad grammar in comment "sub-classed by to"
>>>>>>
>>>>>>
>>>>>> IndexRedirectWriter
>>>>>> line 81: it's an unrelated change, but looks OK
>>>>>>
>>>>>>
>>>>>> ModuleIndexWriter
>>>>>> old lines 98-106: please explain why you have deleted these lines
>>>>>> they don't look immediately related to the Frames support.
>>>>>>
>>>>>> standard.properties
>>>>>> The message needs fixing. The user has asked for "no frames"
>>>>>> and the message is about "frames": i.e. it's not directly related
>>>>>> to what the user did. I suggest:
>>>>>> 445 doclet.NoFrames_specified=\
>>>>>> 446 The --no-frames option is no longer required and may be removed\n
>>>>>> 447 in a future release.
>>>>>>
>>>>>> DocPaths.java
>>>>>> I'm slightly surprised there were quite so many variants of 
>>>>>> allclasses*.html files!
>>>>>>
>>>>>> TestGeneratedBY.java
>>>>>> You've commented out a test ...
>>>>>>
>>>>>> -- Jon
>>>>
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/javadoc-dev/attachments/20190329/75d4d225/attachment.html>