Priya,
I do not think it is worth adding a 3rd kind of stylesheet to Head.java,
to "paper over" the differences between local-relative and root-relative
paths to stylesheets. It should be easy enough to use a single
form/policy for all stylesheets, main and additional, and to cover all
bases, it would seem that root-relative arguments for Head.java might
make more sense, so that clients do not have to convert the
root-relative values for the main stylesheet and additional stylesheets
specified on the command line. In addition, other DocPaths used in
Head.java are also root-relative, so it might be good to make that a
documented rule.
That then reduces to the issue of how to accommodate the module-specific
and package-specific stylesheets. In your webrev, it looks like you are
computing local-relative paths, by the use of DocPaths.forRoot on this
line in HtmlDocletWriter:
2233 DocPath basePath = DocPaths.forRoot(pkgElement);
and is also affected by this code in DocFilesHandlerImpl
131 if (srcFile.getName().endsWith(".css"))
132
stylesheets.add(DocPaths.DOC_FILES.resolve(srcFile.getName()));
So, I think you could do one of the following:
1. Convert these two places to generate root-relative paths.
For line 2233, you could use DocPaths.forPackage instead of
DocPaths.forRoot.
For the other, you would need something like
DocPaths.forRoot(((PackageELement)
element).resolve(DocPaths.DOC_FILES).resolve(srcFile.getName()
2. Or, continue to use local-relative paths for getLocalStyleSheets,
and convert them to root-relative in HtmlDocletWriter:444-448.
Either way, you have to either change command-line stylesheets or local
stylesheets to make them follow the same policy.
-- Jon
On 05/13/2019 01:24 AM, Priya Lakshmi Muthuswamy wrote:
Hi Jon,
I will change the name for method getLocalStylesheetContent.
Regarding the additionalStylesheets:
I am passing both the package/module local stylesheets present in
doc-files and additional stylesheet( specified via --add-stylesheet)
to Head.setStylesheets.
Both main and additional stylesheets are present in root of the
generated doc hierarchy.
For package/module local stylesheets, since they are used only by the
local package/module files, i feel relative path from current file is
better than relative path from root
of the doc hierarchy.
getLocalStylesheets should get path for module/package doc-files and
pathToRoot.resolve will inverse and add to
it(../../ma/doc-files/spanstyle.css).
Another way would be to change Head.setStylesheets method to take
three arguments.
mainStylesheet, additionalStylesheet, package/module local stylesheets.
For main and additional stylesheets, relative path can be calculated
inside Head.java(pathToRoot.resolve) and package/module local
stylesheets paths can be included directly.
Kindly let me know on this.
Regards,
Priya
On 5/11/2019 5:29 AM, Jonathan Gibbons wrote:
The name getLocalStylesheetContent is now inappropriate since it no
longer returns any Content.
I suggest using just getLocalStylesheets.
I see you are changing the type for getAdditionalStyleSheets in
HtmlConfiguration.java and Head.java.
I think I can see why you are doing that, but it is inconsistent to
be changing the "additional stylesheets"
from using DocFile to DocPath, but then not changing the "main
stylesheet" as well. You should either
neither or change both.
In addition, if you change from using DocFile to DocPath, for both
the main and additional stylesheets,
since DocPath's are relative paths you should add a comment to
Head.setStylesheets to specify
whether the DocPaths should be relative to the root of the generated
doc hierarchy or whether they
should be relative to the current file being generated. I don't have
a strong opinion which of the two it
should be, but you should document it, and then make sure the
implementation follows that rule.
(FWIW, my intuition says that the paths should be specified relative
to the root of the document
hierarchy, meaning that you will need to use pathToRoot.resolve
_inside_ Head .java and not _outside_.)
-- Jon
On 05/10/2019 04:32 AM, Priya Lakshmi Muthuswamy wrote:
Hi Jon,
Have modified the code to set the style sheets via
Head.setStylesheets. Method getLocalStyleSheetContent will return
List<DocPath>.
updated webrev:
http://cr.openjdk.java.net/~pmuthuswamy/8219313/webrev.01/
<http://cr.openjdk.java.net/%7Epmuthuswamy/8219313/webrev.01/>
Thanks,
Priya
On 5/10/2019 6:34 AM, Jonathan Gibbons wrote:
The underlying mechanism of getLocalStyleSheetContent is generally
wrong.
The code should not be creating content containing LINK elements,
when the code to do that is in Head.java. The code should be
creating a List<DocPath> containing pointers to all the stylesheet
files, and should then be using Head.setStylesheets, which allows a
list of additional files to be passed in.
Also, in the test, can you put a couple of packages in the module,
with different depths (e.g. package "p" and package "p.q") to
ensure that the relative links work correctly in more cases.
-- Jon
On 04/29/2019 09:21 PM, Priya Lakshmi Muthuswamy wrote:
Hi Jon,
getModuleStylesheetContent is used to get the relative path of
style-sheets in doc-files directory of the corresponding module.
In the case of packages/class files, we pass the PackageElement to
the getLocalStylesheetContent,
This method checks for the style-sheet in both the module and
package doc-files directory.
getModuleStylesheetContent method checks whether there is a named
module and returns the relative path of the style-sheet.(Ex:
../doc-files/mod-stylesheet.css)
In the case of module files, we pass the ModuleElement to the
getLocalStylesheetContent, which just needs to check for
stylesheet in the local doc-files directory(Ex:
doc-files/mod-stylesheet.css)
Thanks,
Priya
On 4/29/2019 9:15 PM, Jonathan Gibbons wrote:
At first glance, this looks questionable.
2193 if (element instanceof PackageElement) {
2194
stylesheetContent.add(getModuleStylesheetContent((PackageElement)element));
2195 }
Is this supposed to be using PackageElement and not ModuleElement?
-- Jon
On 4/29/19 2:32 AM, Priya Lakshmi Muthuswamy wrote:
Hi,
Kindly review the changes for supporting module specific style
sheets.
JBS: https://bugs.openjdk.java.net/browse/JDK-8219313
webrev: http://cr.openjdk.java.net/~pmuthuswamy/8219313/webrev.00/
Thanks,
Priya
