RFR 8131019: jshell tool: access javadoc from tool

[Jonathan Gibbons]

Looks OK to me.

-- Jon

On 10/26/2016 10:35 AM, Jan Lahoda wrote:
> Thanks for all the comments so far. An updated webrev reflecting the 
> comments is here:
> http://cr.openjdk.java.net/~jlahoda/8131019/webrev.01/
>
> Also needs this change to the top-level repository:
> http://cr.openjdk.java.net/~jlahoda/8131019/toplevel.01/
>
> Summary of changes:
> -the text adjustment suggested by Robert
> -the output is paginated, and pauses after methods (if another method 
> follows)
> -most of the comments below have been addressed (some more comments 
> inlined).
>
> On 26.10.2016 04:48, Jonathan Gibbons wrote:
>>
>>
>> On 10/17/2016 09:08 AM, Jan Lahoda wrote:
>>> Hello,
>>>
>>> This patch adds ability to show javadoc inside JShell (by showingg it
>>> on the second invocation of the Shift-<tab> documentation). In
>>> addition to jdk.jshell changes, there is a support in jdk.compiler,
>>> that is expected to be reused by jjs.
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8131019
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~jlahoda/8131019/webrev.00/
>>>
>>> Any feedback is welcome!
>>>
>>> Thanks,
>>>     Jan
>>
>> The file
>> src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javadocformatter.properties 
>>
>>
>> doesn't seem like it belongs in com/sun/tools/javac/resources.
>
> Moved to:
> src/jdk.compiler/share/classes/jdk/internal/shellsupport/doc/resources/javadocformatter.properties 
>
>
>>
>> It would be nice to provide and use String constants for the ANSI escape
>> sequences.
>
> Using constants now.
>
>>
>> JavadocFormatter.java:113 -- ex.printStackTrace seems suspect,
>> especially as there seems
>> to be better infrastructure for handling unwanted exceptions elsewhere
>> in JShell.
>
> Should not normally happen, I think, so I changed that to throw 
> InternalError.
>
>>
>> JavadocFormatter.java:183 -- no distinguished handling for type 
>> parameters
>
> Fixed.
>
>>
>> JavadocFormatter.java:240 -- is it safe to assume lower case tags at
>> this point?  If it helps,
>> you could leverage com.sun.tools.doclint.HtmlTag
>
> Using HtmlTag now.
>
>>
>> JavadocFormatter.java:356 -- (general comment) what about invalid HTML
>> input.
>> Yes, JDK javadoc comments are reasonably good these days, but that
>> doesn't apply
>> everywhere.
>
> I tried to fix the formatter so that it does not crash on broken HTML. 
> The output may not be ideal, though.
>
>>
>> JavadocFormatter.java:333 -- no explicit handling for </p>
>
> Is it necessary to handle the closing tag?
>
>>
>> JavadocFormatter.java:444 -- suggest moving resource file
>> Just curious, why the CAP prefix?
>
> That's for "caption".
>
>>
>> JavaDocFormatter.java:visitEndElement: it is common to see optional end
>> tags omitted, like
>>      </li>, </td>, </tr>
>>      This could screw up the stacks. If </tr> is omitted, won't you skip
>> printing the row?
>
> I've tried to fix the "tr" problem. Added tests for missing closing 
> "td" (there were existing tests for missing closing "li").
>
>>
>> JavadocFormatter.java:409
>>      Ignoring </td>, </th> will mean that vertical whitespace between
>> table cell elements
>>      may be incorporated into the table cell itself, won't it? I guess
>> in general, it'll all be
>>      collapsed into a space character, so in general you'll be OK
>
> I tried to clear out the whitespaces so that these should get 
> eliminated (but they would probably not be a big deal).
>
>>
>> JavadocFormatter.java
>>      No support for entities, including < > &
>
> Done.
>
>>
>> JavadocHelper.java
>>      I haven't figured out how the {@inheritDoc} resolver is working,
>> insofar as how are you
>>      working with the issue that you have to create a new JavacTask for
>> each tree that you
>>      parse, which means separate distinct components like Names, Symtab,
>> and all the other
>>      javac internal classes. In particular, when you start computing
>> supertypes, how do
>>      you coordinate the different elements provided by the different
>> JavacTask objects?
>
> Mapping from the original JavacTask to the new one is done using a 
> "handle" (the "elementSignature" will produce a textual representation 
> of an Element which is found in the newly parsed source). The 
> resulting javadoc is a plain text (with @inheritDoc resolved).
>
> Any feedback is welcome.
>
> Thanks!
>
> Jan
>
>
>>
>> -- Jon