RFR [16] 8251550: Clean up jdk.javadoc and the related parts of jdk.compiler

[Jonathan Gibbons]

ErroreousTree:32
delete "a" before "malformed text"

This is good; thanks for doing this.  I still see places where we don't 
use {@code} around type names, or are inconsistent in the HTML 
typography around variables names, arising from the general tension of 
whether to make the presented form more consistent or the source code 
easier to read. But that's a different bikeshed half way down a 
different slippery slope that can wait until another day.

-- Jon

On 8/17/20 7:28 AM, Jonathan Gibbons wrote:
> I've read your responses, which all look OK.
>
> I'll give the webrev a look as well.
>
> -- Jon
>
> On 8/17/20 5:30 AM, Pavel Rappo wrote:
>> Thanks for having a good look at it, Jon. My replies are inline.
>>
>>> On 14 Aug 2020, at 17:49, Jonathan Gibbons 
>>> <jonathan.gibbons at oracle.com> wrote:
>>>
>>> Good cleanup.
>>>
>>> Some systemic changes needed. Details below.
>>>
>>> -- Jon
>>>
>>> On 8/13/20 11:48 AM, Pavel Rappo wrote:
>>>> Hello,
>>>>
>>>> Please review the below change for
>>>> https://bugs.openjdk.java.net/browse/JDK-8251550
>>>>
>>>>
>>>>    http://cr.openjdk.java.net/~prappo/8251550/webrev.00/
>>>>
>>>>
>>>> This represents squashed commits that have accumulated in my git 
>>>> branch. Since the changeset has started to look dangerously big, I 
>>>> decided not to wait until we finish the migration to Git and 
>>>> GitHub, and push it sooner. The less the others have to merge, the 
>>>> better.
>>>>
>>>> -Pavel
>>>>
>>>>
>>>
>>> Some of the doctree changes contain this sequence:
>>>
>>>      <p>
>>>      <pre>
>>>
>>> That will trigger a doccheck warning for a redundant <p> tag.
>> Fixed.
>>
>>> Since you're improving the doc comments for the DocTree nodes, it 
>>> would be nice to use <i>...</i> or (better) <var>,,,</var> around 
>>> the variables in the templates.  For example:
>>>
>>>    34  * <pre>
>>>    35  *    @author <var>name-text</var>
>>>    36  * </pre>
>>>
>>>    37  *
>> Understood. However, for now I would prefer consistency (with the 
>> interfaces of the com.sun.source.tree package) over further improvement.
>>
>>> DocRootTree:
>>>
>>> I think you need {@literal ...} on line 33.
>> Why? The doc comment uses an HTML entity, not a naked "@" symbol:
>>
>>      * <p>
>>      * <pre>
>>      *    {@docroot}
>>      * </pre>
>>
>> Are you looking at the actual *diff* rather than at a *view* of the 
>> webrev? Webrev is infamous for producing a certain type of illusions.
>>
>>> DocTypeTree:
>>> As an enhancement, given that the tools only support HTML 5 these 
>>> days, It might be helpful to give the specific text for an HTML 5 
>>> document
>>>
>>>    32  * <pre>
>>>
>>>    33  *    <!doctype text>
>>>
>>>    34  * </pre>
>>>      + * For HTML5 documents, the correct form is  {@code <!doctype 
>>> html>}.
>> Added the proposed line.
>>
>>> EntityTree:
>>> Maybe remove the "misleading" spaces in the examples?
>> Removed the spaces.
>>
>>> General:
>>>
>>> Thinking aloud, for a future update when we have the `{@spec}` tag. 
>>> In the introductory sentence for each tag, replace the use of 
>>> `{@code}` by `{@spec}` linking to the right place in the doc-comment 
>>> tag specification. I guess for many, it's not necessary, but the 
>>> idea came to mind reading IndexTree, where more information may be 
>>> helpful. That being said, additional info belongs in the tag spec, 
>>> not the tree node spec.
>> Agreed.
>>
>>> DocSourcePositions:57,89
>>> replace `CompilationUnit` with `compilation unit`
>> Replaced.
>>
>>> DocTreeFactory:112
>>> We are inconsistent about whether to use `{..}` for inline tags, 
>>> which will be worse when we add our first bimodal standard tag, 
>>> `@return`, but using `{...}` for `{@deprecated}` is definitely wrong
>> Yes, I have that thought too. I fixed "@deprecated" along with other 
>> block tags in DocTreeFactory.java. On the other hand, I added missing 
>> curly braces around "@summary".
>>
>>> General: (question)
>>> Does a trailing space inside `{@code something }` cause extra 
>>> whitespace in the rendered HTML?
>> Yes, it does. I removed those *outer* trailing whitespace characters.
>>
>>> DocTreePath:37,96,98,99
>>> More un-code-d type names. Follow existing conventions for either 
>>> using `{@code...}` or the descriptive equivalent (e.g. "tree path")
>> @code-d those and some others.
>>
>>> Trees:239,240
>>> replace "The" by "the"
>> Replaced there and in one other place.
>>
>>> remove package name from TypeMirror, and use @code for that and 
>>> ErrorType
>> Removed the package, but will NOT do the rest: it would be completely 
>> inconsistent with everything else in that file.
>>
>>> DocCommentParser:
>>> avoid Latin abbreviations; use "for example, such as"
>> Fixed. Although, I don't get the rationale behind this piece of 
>> advice. One reason not to use any abbreviations would be the period 
>> characters: periods interfere with detecting first sentences in doc 
>> comments.
>>
>>> 1103: another candidate for `{@spec}` !!
>> Yes. I just felt I have to provide something more fresh and 
>> immediately accessible.
>>
>>> Pretty: removing import
>>> In general, this is a dangerous edit; throughout javac, you will see 
>>> imports of javac.util.List in preference to java.util.List. I assume 
>>> you have compiled and run the tests, to validate this change.
>> Correct, before posting the initial webrev I ran the tiers 1 through 
>> 3 in our CI. It was fine.
>>
>>> WriterFactory:135
>>> Trailing period.
>> Removed there and in some other places.
>>
>>> BaseTaglet
>>> Yay for removing redundant doc comments from overridden methods!
>>>
>>> toolkit/taglets/Taglet.java
>>> 41,143:
>>> If you think "tag" refers to instances in doc comments, the original 
>>> was correct
>>> If you think "tag" refers to the class of the instances, the new 
>>> text should use "the block tag"
>> Those terms have to be better defined in the spec.
>>
>>> 142: consider remove "all" from end of line
>> Removed.
>>
>> -Pavel
>>
>>> -- Jon
>>>