RFR : 8213354 : Support package-specific stylesheets

[Priya Lakshmi Muthuswamy]

Hi Jon,

Thanks for the review.

I have modified the DocFilesHandlerImpl to get all the .css files, so 
that user is not restricted to one name.
Also included the Map in HtmlConfiguration.

updated webrev : http://cr.openjdk.java.net/~pmuthuswamy/8213354/webrev.01/

Thanks,
Priya

On 2/9/2019 6:24 AM, Jonathan Gibbons wrote:
> Hi Priya,
>
> There are two significant issues that need to be addressed.
>
> 1.
>
> In DocFilesHandlerImpl you randomly take the first .css file you find 
> when listing the directory. If there should only be one, we should 
> specify the name (e.g. stylesheet.css). Otherwise, I think you should 
> honor all files found. Note that the order of iteration of a directory 
> is undefined, and may depend on the operating system, and may vary 
> over time. That is a very strong reason not to just take the first 
> file found when listing the directory.
>
> In terms of naming, "check..." is not a good name for this method. I'd 
> suggest either getStylesheet (if you only want to handle one) or 
> getStyleSheets() returning a List<DocPath> if you support more than 
> one.  Note that using a list allows you to easily handle no files 
> found (i.e. an empty list) as well as 1 or more files found.
>
> Minor nit: please ensure a space between a keyword and an open 
> parenthesis. You should be able to configure your IDE for that style; 
> it is the standard style for JDK code.
>
> 2.
>
> You should not be modifying anything for this feature in the 
> jdk/javadoc/internal/doclets/toolkit/ directory. The use of 
> stylesheets and CSS is specific to the HTML format, and any changes 
> should be limited to the directory 
> jdk/javadoc/internal/doclets/formats/html/ (and any subdirectories.)
>
> I realize you need to get information into the individual *WriterImpl 
> classes.  In general, the way we do that is to build and use a table 
> in the HtmlConfiguration object. Some class like HtmlDoclet can stash 
> information in that table; other Html classes can access it.  The 
> table can either be a Map<PackageElement,DocPath> or a 
> Map<PackageElement,List<DocPath>> depending on whether you support at 
> most one stylesheet per package or many.
>
> If you keep a Map in HtmlConfiguration, you could also compute entries 
> lazily, and not bother with editing HtmlDoclet. For example, no entry 
> in the cache means it has not been computed yet, an empty list in the 
> cache means no package-specific stylesheets, a non-empty list means 
> you have local stylesheets. This computation can all be done in 
> DocFilesHandlerImpl, with just the cache object itself being retained 
> in HtmlConfiguration.
>
> -- Jon
>
>
> On 01/30/2019 04:18 AM, Priya Lakshmi Muthuswamy wrote:
>> Hi,
>>
>> Kindly review the fix for 
>> https://bugs.openjdk.java.net/browse/JDK-8213354
>> Package specific stylesheets needs to placed under doc-files 
>> directory of a package.
>> DocFilesHandler looks for any css file and adds them to the generated 
>> html pages.
>>
>> webrev : http://cr.openjdk.java.net/~pmuthuswamy/8213354/webrev.00/
>>
>> Thanks,
>> Priya
>>
>