RFR: 8214738 : javadoc should honor styles in doc-files

Thu Dec 20 21:46:18 UTC 2018

Priya,

In DocFilesHandlerImpl, you have a boolean flag titleOrMetaTags. Since 
<meta> elements never have content, and so never have end elements, it 
seems misleading/incorrect to set the flag to true.  Also, you omit to 
handle ENTITY in the <title> element, so I added that in as well.  You 
could also write this code as a visitor if you wanted, but the switch 
form is not too bad.

I suggest the code should look something like this:

205 List<DocTree> localTags = new ArrayList<>();  206 boolean inHead = 
false;
207 boolean inTitle = false; loop:   208 for (DocTree dt : dtrees) {
209 switch (dt.getKind()) {
210 case START_ELEMENT:
211 StartElementTree startElem = (StartElementTree) dt; switch 
(HtmlTag.get(startElem)) { case HEAD: inHead = true; break;case META: 
break; case TITLE: inTitle=true; break; default: if (inHead) { 
localTags.add(startElem); } }
223 break;   224 case END_ELEMENT:  211 EndElementTree endElem = (EndElementTree) dt; 
switch (HtmlTag.get(endElem)) { case HEAD: break loop;                               case TITLE:
                                  inTitle = false;
                                  break loop;default: if (inHead) { localTags.add(startElem); } }
235 break; case ENTITY:   236 case TEXT:
237 if (inHead && !inTitle) {
238 localTags.add(dt);
  239                     }
240 break;
  241             }

-- Jon

On 12/20/2018 11:31 AM, Jonathan Gibbons wrote:
>
>
>
> On 12/19/2018 08:38 PM, Priya Lakshmi Muthuswamy wrote:
>> Hi,
>>
>> Kindly review the fix for 
>> https://bugs.openjdk.java.net/browse/JDK-8214738
>> webrev : http://cr.openjdk.java.net/~pmuthuswamy/8214738/webrev.00/
>>
>> Thanks,
>> Priya
>>
>>
>
> I don't think you need to add anything (userHeaderTags etc) to Head.
> Head already has the requisite feature, "extraContent" and "addContent"
> which can be used to add content into the <head> element.
>
> Re: 
> Utils.toLowerCase(nodeEnd.getName().toString()).equals(HtmlTag.HEAD.getText())
> Don't use long-winded String comparison when you have type-safe 
> comparison.
> In this case, you should be able to do something like
>     HtmlTag.get(nodeEnd.getName()) == HtmlTag.HEAD
>
> getLocalHeaderTags needlessly walks through all the <body> element. 
> You can
> break out of the loop once you hit </head>.
>
> DocFilesHandlerImpl:196.
> The "if" statement is a waste of time and effort. The code will work 
> perfectly
> well when localTagsContent is an empty list, so you can simplify 
> 196-199 to just 197.
>
> TestCopyFiles.java
> (style) re: indenting the continuation of multi-line strings
> The general Java style for breaking long expressions is to break 
> before a binary operator, not after.
> The style in other javadoc tests is to not indent the continuation 
> expressions, so that it is easier
> to see how the first line and subsequent lines are aligned.
>
> -- Jon

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/javadoc-dev/attachments/20181220/f1c5e784/attachment.html>