RFR:8185985:Html files in doc-files directories should be wrapped

[Kumar Srinivasan]

Jon,

> DocletElement:84
>
>   83      * Returns the anchoring package element, in the case of a
>   84      * module element, this could be an unnamed package.
>
> Change to:     this is the module's unnamed package

Fixed

> DocFilesHandlerImpl
>
>  199         sb.trimToSize();
>  200         return docletWriter.getWindowTitle(sb.toString().trim());
>
> No need for line 199; remove it

Fixed

>
> DocFilesHandler.java
>
>   39     void copyDocFiles() throws DocletException ;
>
> Space before ';'

Fixed

> It would be nice if the MODULE and PACKAGE links were active.

Done and thanks for the override tip!.

Also added more tests for single and multi modules, the latter the cause 
behind
links not appearing.

The corrected doc-file:
http://cr.openjdk.java.net/~ksrini/8185985/api-docs/api/java/util/doc-files/coll-index.html

Delta webrev:
http://cr.openjdk.java.net/~ksrini/8185985/webrev.02/webrev.delta/


Thanks
Kumar




>
> -- Jon
>
>
> On 11/09/2017 03:09 PM, Kumar Srinivasan wrote:
>> Hi Jon,
>>
>> [changed subj line to reflect the current review]
>> Full webrev:
>> http://cr.openjdk.java.net/~ksrini/8185985/webrev.01/
>>
>> Delta webrev:
>> http://cr.openjdk.java.net/~ksrini/8185985/webrev.01/webrev.delta/
>>
>>> Here's comments on the code (not looked at the tests yet).
>>>
>>> On 11/06/2017 06:04 PM, Kumar Srinivasan wrote:
>>>>
>>>> [3] https://bugs.openjdk.java.net/browse/JDK-8185985
>>>> [4] http://cr.openjdk.java.net/~ksrini/8185985/webrev.00/ 
>>>
>>> HtmlDocletWriter: line 451-493
>>> I accept you have put the new code after the existing like-named 
>>> method,
>>> but we're still trying to de-clutter classes like HtmlDocletWriter.
>>> The new method seems like a combination of two methods ...
>>> one is new functionality to extract the TITLE from an HTML document,
>>> and the other is a simple call onto the existing getWindowTitle.
>>>
>>> I'm also not sure that the filename is a good default. But that 
>>> being said,
>>> TITLE is not an optional element in a HEAD section, and should 
>>> always be
>>> provided, and so a non-empty default may never normally be required.
>>
>>  I have changed it to  use an empty string.
>>
>>
>>>
>>>
>>> HtmlDocletWriter: line 2495,2496
>>> Suggest increasing indent of parameter list
>>
>> Done
>>
>>>
>>> WriterFactoryImpl: 244
>>> You're in a class called "WriterFactoryImpl" and the method is named 
>>> "getDocFilesWriter"
>>> and yet it returns an object whose type is DocFilesHandler, instead 
>>> of DocFilesWriter.
>>> Just sayin'....
>>
>> Fixed
>>
>>>
>>> BaseConfiguration:378
>>> Were you going to move this assignment later in the execution?
>>
>> Done
>>
>>
>>>
>>> CommentUtils:183
>>> When is the returned type not a PackageElement?
>>> Why is the return type of the method not PackageElement?
>>> If it can be something else, like a module element, should there be 
>>> a check here?
>>
>> Fixed
>>>
>>> CommentUtil:219-221
>>> The comment requires some mental gymnastics to guess any possible 
>>> meaning.
>>> If you know what it means, can you rewrite it please?
>> Fixed
>>
>>>
>>> OverviewElement
>>> We definitely talked about removing any direct dependence on and 
>>> storage of
>>> BaseConfiguration.
>>
>> Fixed
>>
>>>
>>> WriterFactory
>>> See comments for WriterFactoryImpl re: naming.
>>>
>>> AnnotationTypeBuilder:31
>>> ClassBuilder:31
>>> ModuleSummaryBuilder:30
>>> PackageSummaryBuilder:34
>>> Illegal import. builders cannot depend on types in format-specific 
>>> packages
>>> (i.e. formats.html)  There needs to be an interface in the toolkit 
>>> world, and
>>> an impl of the interface in the formats.html world, the same as is 
>>> done for
>>> other Writer/WriterImpl pairs.
>>
>> Done
>>
>>>
>>> ModuleSummaryBuilder:120
>>> You're very close to being able to support this. The PackageElement for
>>> the DocletElement would be the unnamed package for the module.
>>
>> It all seems to work, for modules, however I have disabled it for 
>> now, added suitable
>> comments.
>>
>>>
>>> DocFile:106
>>> Grumble, why do you need this?  In general, the doclet world uses 
>>> DocFile objects
>>> not FileObjects
>>
>> FO needed  for DocTrees.
>>
>>>
>>> DocPaths:173-196
>>> This class used to be sorted in case-equivalent alpha order. 
>>> Somehow, these 'M'
>>> fields and methods have ended up between 'P' and 'R'
>>
>> Fixed.
>>
>>>
>>> StandardDocFileFactory
>>> Again grumble that this is necessary, but I can see where this going
>>>
>>> Utils
>>> Who is now using these copyDirectory methods, since they don't check 
>>> for HTML files?
>>
>> The doclet support files such as resources, jquery etc. Howeve please 
>> note the doc-files
>> are copied by its own copyDirectory logic.
>>
>>>
>>> DocFilesHandler.java
>>> What about .HtMl ... in other words, get the extension and do a 
>>> ignore-case check.
>>> line 153: in time we'll want more from the <head> than just the 
>>> title, so I think
>>> getTitle belongs here.  Also, I don't think the title gets properly 
>>> propagated to
>>> the TITLE in the generated file.
>>
>> moved it here and converted the file extension check as suggested. 
>> And it seems
>> to be propagated to the output, there are tests to check this.
>>
>>>
>>> DocFileElement:64
>>> You want the unnamed package for the specific module (each module 
>>> has its own unnamed package.)
>>
>> added a comment for future use.
>>
>>>
>>> DocletElement:83,84
>>> You can't put types in the unnamed package of a named module, but 
>>> javac (Symbol, etc)
>>> does understand the concept, and so it is reasonable (in the API) to 
>>> get the unnamed package
>>> for a named module. You might need a workaround to get it, though.
>>
>> Fixed comment
>>
>> Thanks
>> Kumar
>>
>>>
>>>
>>> -- Jon
>>
>