RFR 8167636: jshell tool: Edit Pad should be in its own module

[Robert Field]

Hi Rémi,

On 10/23/16 00:47, Remi Forax wrote:
> Hi Robert,
> in ConsoleIOContext, the private bind() can take a Consumer (or a Runnable) as second parameter so you don't need to use a special functional interface for that (ActionListener).

Saw your response after I fixed the ConsoleIOContext dependencies-- 
including adding support for Consumer (which isn't in our version of 
JLine) and then noticing Runnable ;-)

>
> BuildInEditorProvider should be an interface, the only reason to not use an interface as a module service is backward compatibility.

OK.
All the examples I saw used abstract classes, so I assumed there was 
some reason.

>
> In jdk.shell module, i don't know why you restrict the access to jdk.internal.jshell.editpad. If someone want it's own editor, why not letting him to implement BuildInEditorProvider.

Allowing alternative implementations of the built-in editor is certainly 
a reasonable thing -- though, probably the external editor choice is 
much easier -- the built-in editor doesn't, at this time, really add 
anything that an external editor doesn't, except that it is built-in.  I 
didn't want to bake what the interface to it would be at this time.

>
> EditPane do not need to be a JFrame (i.e. inherits from a JFrame), you can create a plain old JFrame in run().
> Also the CountDownLatch::countDown can be shield behind a Runnable so only EditPadProvider knows that it uses a CountDownLatch, the EditPane will only call the Runnable.

Will look at that.

Thanks much for the comments!

-Robert

>
> cheers,
> Rémi
>
> ----- Mail original -----
>> De: "Robert Field" <robert.field at oracle.com>
>> À: "Kulla" <kulla-dev at openjdk.java.net>
>> Envoyé: Dimanche 23 Octobre 2016 08:52:28
>> Objet: Re: RFR 8167636: jshell tool: Edit Pad should be in its own module
>> Note: jdk.internal.le and thus the jshell tool's ConsoleIOContext have a
>> dependency on java.awt.event.ActionListener.
>> The former uses reflection to quietly ignore when it isn't present -- at
>> a loss of functionality (functionality we use).
>> This code has a hack until I can discuss with Jan what the options are.
>> Ideally jdk.internal.le can lose it's dependency; Otherwise, I will
>> change ConsoleIOContext to use reflection, which will mean a loss of
>> shift-tab and shortcut+* functionality when java.desktop is not in the
>> image.
>>
>> -Robert
>>
>>
>> On 10/22/16 21:14, Robert Field wrote:
>>> Remove the dependency of the jshell tool on the java.desktop module.
>>> Uses modules and ServiceLoader so that Edit Pad is optional -- that
>>> is, a jlink'ed image can be produced without Edit Pad and without
>>> java.desktop
>>>
>>> Bug:
>>>      https://bugs.openjdk.java.net/browse/JDK-8167636
>>>
>>> Webrev:
>>>      http://cr.openjdk.java.net/~rfield/8167636v0.webrev/
>>>
>>> Thanks,
>>> Robert