RFR 8145263: JShell API: Change the format of SourceCodeAnalysis#documentation

Jan Lahoda jan.lahoda at oracle.com
Mon Oct 3 20:04:13 UTC 2016


Looks OK to me too. I don't have a strong opinion on the throws clause.

Jan

On 30.9.2016 17:26, Robert Field wrote:
> Nice!
>
> Includes a lot of touches to make it more readable and useful.
>
> Passing on one bit of feedback from Brian, put into my words: since we
> are going to be adding full javadoc access, we want this signature
> output crisp. So, the throws clause is probably overkill.
>
> Thanks,
> Robert
>
> On September 30, 2016 4:04:03 AM ShinyaYoshida <bitterfoxc at gmail.com> wrote:
>
>> Hi Robert and Jan,
>> I've updated the webrev to current code base:
>> http://cr.openjdk.java.net/~shinyafox/kulla/8145263/webrev.10/
>>
>> Could you review this?
>>
>> Regards,
>> shinyafox(Shinya Yoshida)
>>
>>
>> 2016-09-27 4:51 GMT+09:00 ShinyaYoshida <bitterfoxc at gmail.com
>> <mailto:bitterfoxc at gmail.com>>:
>>
>>     Hi Robert,
>>     Never mind! And I'm also sorry for having left this.
>>
>>     I'll try updating webrev to current code base until 1/Oct.
>>
>>     BTW, currently a lot of things in jshell are configurable, could
>>     signature of documentation also be configurable in future(JDK10 or
>>     9.1 or ...)?
>>
>>     Thank you,
>>     shinyafox(Shinya Yoshida)
>>
>>     2016-09-26 12:41 GMT-07:00 Robert Field <robert.field at oracle.com
>>     <mailto:robert.field at oracle.com>>:
>>
>>         In reviewing outstanding issues, we discovered this RFR which
>>         was left hanging.
>>
>>         Our sincere apologies for dropping the ball on this.
>>
>>         We are juggling a lot, if we miss something like this in the
>>         future, please let us know.
>>
>>         I have made some changes in the issue, please note them.  I
>>         know there have been some underlying changes as well
>>         (parameter names from source).
>>
>>         If you would be willing to update this RFR we will review
>>         promptly.
>>
>>         Thank you and sorry,
>>         Robert
>>
>>
>>         On 12/15/15 17:07, ShinyaYoshida wrote:
>>>         Hi Jan and Robert,
>>>         Thank you.
>>>
>>>         I've filed:
>>>         https://bugs.openjdk.java.net/browse/JDK-8145473
>>>         <https://bugs.openjdk.java.net/browse/JDK-8145473>
>>>
>>>         Ok, I put the type parameters for the constructor before the
>>>         traditional(current) form:
>>>         http://cr.openjdk.java.net/~shinyafox/kulla/8145263/webrev.01/ <http://cr.openjdk.java.net/%7Eshinyafox/kulla/8145263/webrev.01/>
>>>
>>>         Please review it again.
>>>
>>>         Regards,
>>>         shinyafox(Shinya Yoshida)
>>>
>>>
>>>         2015-12-16 5:56 GMT+09:00 Jan Lahoda <jan.lahoda at oracle.com
>>>         <mailto:jan.lahoda at oracle.com>>:
>>>
>>>             Hi Shinya,
>>>
>>>             On 14.12.2015 15:40, ShinyaYoshida wrote:
>>>
>>>                 Hi Jan,
>>>                 Thank you for your review.
>>>
>>>                 2015-12-14 23:24 GMT+09:00 Jan Lahoda
>>>                 <jan.lahoda at oracle.com <mailto:jan.lahoda at oracle.com>
>>>                 <mailto:jan.lahoda at oracle.com
>>>                 <mailto:jan.lahoda at oracle.com>>>:
>>>
>>>                     Hi Shinya,
>>>
>>>                     Generally, looks good, thanks for looking at
>>>                 this! Two comments:
>>>                     -for parameter names, I was hoping we could get
>>>                 them from the
>>>                     sources (if/when available), but we are not doing
>>>                 that now, and
>>>                     hiding synthetic parameter names makes sense to
>>>                 me. So this is OK,
>>>                     and if we at some point start to parse parameter
>>>                 names from the
>>>                     sources, we can tweak the code to do the right thing.
>>>
>>>                 I think that there should be the issue for the
>>>                 parameter names.
>>>                 Do you have the issue for that?
>>>
>>>
>>>             No issue for this yet.
>>>
>>>                 If not, should I create it?
>>>
>>>
>>>             Sure, thanks.
>>>
>>>
>>>
>>>                     -not sure if marking constructors with ".new"
>>>                     ("type-name.new(<parameters>)") will be clear -
>>>                 do you think the
>>>                     traditional form ("type-name(<parameters>)") is
>>>                 unclear?
>>>
>>>
>>>                 When I consider the constructor with the generics
>>>                 like following, I
>>>                 notice that the traditional(current) form is
>>>                 difficult to represent it.
>>>                 class C<T> { <U> C(U u) {} }
>>>                 So I choose that format which is like the constructor
>>>                 reference.
>>>
>>>
>>>             I think generic constructors (i.e. constructors that
>>>             themselves have type parameters) are very uncommon, so I
>>>             wouldn't optimize for those. Having the format nice for
>>>             usual constructors is more important, I think, even if
>>>             the format for these uncommon constructors would be uglier.
>>>
>>>             Thanks,
>>>                 Jan
>>>
>>>
>>>
>>>                 Another possible representation is "new <Generics>
>>>                 type-name<Generics>(<parameters>)" which is similar
>>>                 to the invocation of
>>>                 the constructor with generics.
>>>
>>>                 What do you think?
>>>
>>>                 Regards,
>>>                 shinyafox(Shinya Yoshida)
>>>
>>>
>>>                     Thanks,
>>>                          Jan
>>>
>>>
>>>                     On 13.12.2015 07:33, ShinyaYoshida wrote:
>>>
>>>                         Hi Jan and Robert,
>>>                         I'd like to propose changing the format of
>>>                         SourceCodeAnalysis#documentation.
>>>
>>>                         The detail of the change is on the issue 8145263:
>>>                 https://bugs.openjdk.java.net/browse/JDK-8145263
>>>                 <https://bugs.openjdk.java.net/browse/JDK-8145263>
>>>                         Please see it.
>>>
>>>                         You can see the more example in the test of
>>>                 my patch.
>>>
>>>                         Patch is here:
>>>                 http://cr.openjdk.java.net/~shinyafox/kulla/8145263/webrev.00/
>>>                 <http://cr.openjdk.java.net/%7Eshinyafox/kulla/8145263/webrev.00/>
>>>
>>>                         Please consider my proposal and review the patch.
>>>
>>>                         Regards,
>>>                         shinyafox(Shinya Yoshida)
>>>
>>>
>>>
>>
>>
>>


More information about the kulla-dev mailing list