RFR : 8219313 : Support module specific stylesheets

[Priya Lakshmi Muthuswamy]

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/20190513/e9d48c49/attachment.html>