RFR: 8154482: javadoc tool must support legacy doclet and taglet

[Jonathan Gibbons]

OK, but one has to wonder why ExitJavadoc is defined in Messager, and not
at the top level.

-- Jon

On 04/29/2016 11:28 AM, Kumar Srinivasan wrote:
>
> Please review, this addresses all your comments below.
> http://cr.openjdk.java.net/~ksrini/8154482/webrev.02/
>
> Thanks
> Kumar
>
>> Start.java, line 241, 241, use &&, not nested if
>>
>> Start:490,  generally, the terminology in langtools is to use "path"
>> to mean a complete search path, as in a series of file system locations,
>> such as a class path or source path.   With that in mind, the decl
>> on 490 would be better named "userTagletPath" (singular)
>> not "userTagletPaths" plural.
>>
>> Yes, it gets confusing in parts of the code that use the NIO2 type
>> Path as a replacement for File.  There, the convention is to use
>> "search path" as the generalization of class path, source path, etc.
>>
>> Start 598, 605, 616
>> The use of panic and its use of Messager.exit would be improved
>> by replacing
>>     panic(key, args)
>>     return false; // keep compiler happy
>> with either
>>     error(key, args)
>>     throw new ExitJavadoc();
>>
>> (Ultimately, Start.exit and Messager.exit should go away.)
>>
>> javadoc.properties.
>> 118.   Ending a sentence "at the earliest." may be an acceptable
>> colloquialism, but guardians of the Queen's English would raise
>> an eyebrow or two.   I would just delete these words.  Yes, we
>> should recommend that folk migrate, but time is not of the
>> essence,
>>
>>
>> new test:
>> It's OK as it is, but the test-case names are not informative,
>> and may be tedious to update if you want  to add a new test case
>> in the middle of the list at some point in the future. Also, the
>> names are uninformative when they show up in a stack trace.
>> With this style of test case, I've typically found it useful to
>> use description names that describe the test case or test
>> conditions.
>>
>> TestRunner should have class level javadoc. Thanks for creating
>> this class.
>>
>> -- Jon
>>
>>
>>
>> On 04/28/2016 10:48 AM, Kumar Srinivasan wrote:
>>>
>>> Hello,
>>>
>>> Based on the reviews here is the second revision of the fix.
>>> http://cr.openjdk.java.net/~ksrini/8154482/webrev.01/
>>>
>>>
>>> Highlights of the changes:
>>>
>>> 1.  Removed the redundant undocumented option -Xnew
>>> 2. A doclet on the command lines decides the target,
>>>     irrespective of the taglet variant, this simplifies the logic.
>>> 3. Added a new annotation based testcase execution utility,
>>>     in the toolbox package.
>>> 4. Added more testcases, refactored the test to use toolbox utilities,
>>>    and other simplifications.
>>>
>>> Thanks
>>> Kumar
>>>
>>>
>>>
>>>> Hello,
>>>>
>>>> Please review  fix for:
>>>> https://bugs.openjdk.java.net/browse/JDK-8154482
>>>>
>>>> Webrev at:
>>>> http://cr.openjdk.java.net/~ksrini/8154482/webrev.00/
>>>>
>>>> Thanks
>>>> Kumar
>>>>
>>>
>>
>