RFR : 8219313 : Support module specific stylesheets
Priya Lakshmi Muthuswamy
priya.lakshmi.muthuswamy at oracle.com
Tue May 14 10:23:45 UTC 2019
Hi Jon,
I have modified the code to set the stylesheets to be relative to root
of the documented hierarchy.
updated webrev : http://cr.openjdk.java.net/~pmuthuswamy/8219313/webrev.02/
Thanks,
Priya
On 5/14/2019 12:36 AM, Jonathan Gibbons wrote:
>
> 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
>>>>>>>>
>>>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/javadoc-dev/attachments/20190514/f294bbda/attachment.html>
More information about the javadoc-dev
mailing list