WIP: 8154714: JShell API: addExports support [for Java 9.X]

[Robert Field]

Thanks shiyafox! This is a significant piece of code!

As to your questions --

 > I wonder if  you could work on the documentation of the API and the 
tool instead?

Yes, I will do API and tool docs.

 > Subcommand of /set is reasonable? Similar command, /classpath, to 
make users available to use new packages is not a subcommand of /set.

Yes, I think /set exports is best.  And, yes, /classpath is 
inconsistent, it should be /set classpath, that can be a separate task, 
unless you want to roll that in.

 > Should we need retain feature to this?

All /set have a /retain counterpart -- we should maintain that


As to the code review: Looks good to me, just minor questions/issues --

JShellTool.java:

* As discussed /set add-exports => /set exports

* Nit: a comment got moved out of place line 1194(old)

JShell.java:

* EXT_CMD_AVAILABLE_MODULES and EXT_CMD_AVAILABLE_PACKAGES, these should 
be referenced via RemoteCodes.java rather than be coped.

* Which leads me to another question: why is addExports a command and 
these two are extensions.  It seems fine, just curious.  I notice that 
extensions are a lot less code -- ;-)

TaskFactory.java:

* Question: the "/" separator between module and package is that 
platform independent?

DefaultLoaderDelegate.java:

* Jan, can you look at this code?


-Robert


On 09/01/16 05:36, ShinyaYoshida wrote:
> Hi all,
> I've implemented this feature to JShell API and Tool with full completion:
> Bugs: https://bugs.openjdk.java.net/browse/JDK-8154714
>
> jdk: 
> http://cr.openjdk.java.net/~shinyafox/kulla/8154714/webrev.00.jdk/ 
> <http://cr.openjdk.java.net/%7Eshinyafox/kulla/8154714/webrev.00.jdk/>
> langtools: 
> http://cr.openjdk.java.net/~shinyafox/kulla/8154714/webrev.00.langtools/ 
> <http://cr.openjdk.java.net/%7Eshinyafox/kulla/8154714/webrev.00.langtools/>
>
> This is missing the train of Java 9.0 but it could catch the train of 
> Java 9.1 or later.
> I want to get your comments for this.
>
> jshell> /set add-exports jdk.jconsole sun.tools.jconsole
>
> jshell> import sun.tools.jconsole.*
>
> jshell> LocalVirtualMachine.getAllVirtualMachines().forEach((k, v) -> 
> System.out.println(k + ":" + v))
> 10778:jdk.jshell/jdk.internal.jshell.tool.JShellTool
> 10826:jdk.jshell.execution.RemoteExecutionControl 35548
> 28447:org.netbeans.Main --cachedir 
> /home/bitter_fox/.cache/netbeans/8.0.1 --userdir 
> /home/bitter_fox/.netbeans/8.0.1 --branding nb
>
> Currently, there is no testing, so it will be required to add test.
>
> Regards,
> shinyafox(Shinya Yoshida)