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
