Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-18 Thread A. Sundararajan
* jdk/make/lib/Lib-jdk.jline.gmk has copyright year 2011, 2015 despite being a new file. Any specific reason? * jdk.jline depends on java.desktop. Is that needed by the code by jline code? I am asking because Nashorn requires only "compact 1" profile so far and so can be used on compact

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-18 Thread Jan Lahoda
On 18.6.2015 16:40, A. Sundararajan wrote: * jdk/make/lib/Lib-jdk.jline.gmk has copyright year 2011, 2015 despite being a new file. Any specific reason? I copied one of the existing files in the directory to just adjusted it for jdk.jline. So I kept the copyright years there - what is t

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-18 Thread A. Sundararajan
My understanding is that the new file won't have old copyright year (2011 in this case). -Sundar On Thursday 18 June 2015 09:20 PM, Jan Lahoda wrote: On 18.6.2015 16:40, A. Sundararajan wrote: * jdk/make/lib/Lib-jdk.jline.gmk has copyright year 2011, 2015 despite being a new file. Any

RE: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-18 Thread Iris Clark
> My understanding is that the new file won't have old copyright year > (2011 in this case). Agreed. Thanks, iris

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-18 Thread Ben Evans
Hi, Has this been tested with a JDK 9 compiler? The last time I checked JLine wouldn't build with an OpenJDK 9 javac. Thanks, Ben On 18 Jun 2015 3:26 pm, "Jan Lahoda" wrote: > Hello, > > I am proposing to add JLine 2.12.1 into the jdk repository for use by the > Java and Nashorn REPLs. Full p

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-22 Thread Erik Joelsson
Hello Jan, Build changes look mostly ok. Some notes: * "$(CFLAGS_WARNINGS_ARE_ERRORS)", which should not be used anymore. Warnings are already treated as errors globally. * There should not be a need to add "-I$(INCLUDEDIR) -I$(JDK_OUTPUTDIR)/include/$(OPENJDK_TARGET_OS)" to cflags. INCLUDED

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-22 Thread Jan Lahoda
Hi Erik, Thanks for the comments. Yes, I copied an existing file, and -DUSE_MMAP seems unnecessary. I'll implement your suggestions. Thanks, Jan On 22.6.2015 10:25, Erik Joelsson wrote: Hello Jan, Build changes look mostly ok. Some notes: * "$(CFLAGS_WARNINGS_ARE_ERRORS)", which should

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-25 Thread Jan Lahoda
Hello, Based on the feedback I've received so far, I've uploaded an updated version of the patch: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.01/full/ Notable changes: -avoided the dependency on java.desktop and java.datatransfer -adjusted the native library build script as per Erik's r

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-25 Thread Alan Bateman
On 25/06/2015 17:25, Jan Lahoda wrote: Hello, Based on the feedback I've received so far, I've uploaded an updated version of the patch: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.01/full/ Notable changes: -avoided the dependency on java.desktop and java.datatransfer -adjusted the n

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-26 Thread Erik Joelsson
Hello Jan, Build changes look good. /Erik On 2015-06-25 18:25, Jan Lahoda wrote: Hello, Based on the feedback I've received so far, I've uploaded an updated version of the patch: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.01/full/ Notable changes: -avoided the dependency on java.de

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-26 Thread Jan Lahoda
Hi Alan, Thanks for the comments! A question inline. On 25.6.2015 18:38, Alan Bateman wrote: On 25/06/2015 17:25, Jan Lahoda wrote: Hello, Based on the feedback I've received so far, I've uploaded an updated version of the patch: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.01/full/

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-26 Thread Alan Bateman
On 26/06/2015 08:58, Jan Lahoda wrote: : The native method readKeyEvent seems to do a FindClass per key event. Maybe this is from the upstream project but I would think it would be more efficient to cache this in a global ref. It would also be more I will work on this. Related to this is

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-26 Thread Jan Lahoda
Uploaded a new iteration of the patch, reflecting the comments so far, here: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.02/full/ A delta patch from previous iteration: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.02/delta/ Any feedback on this would be appreciated! Thanks for the

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-26 Thread Alan Bateman
On 26/06/2015 14:38, Jan Lahoda wrote: Uploaded a new iteration of the patch, reflecting the comments so far, here: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.02/full/ A delta patch from previous iteration: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.02/delta/ Any feedback on th

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-29 Thread Jan Lahoda
On 26.6.2015 16:00, Alan Bateman wrote: On 26/06/2015 14:38, Jan Lahoda wrote: Uploaded a new iteration of the patch, reflecting the comments so far, here: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.02/full/ A delta patch from previous iteration: http://cr.openjdk.java.net/~jlahoda/8080

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-29 Thread Alan Bateman
On 29/06/2015 10:10, Jan Lahoda wrote: Thanks for the comment - done that. Updated webrev: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.03/full/ Delta against the previous iteration: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.03/delta/ How does this look? Feedback is appreciate

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-29 Thread Jan Lahoda
On 29.6.2015 12:09, Alan Bateman wrote: On 29/06/2015 10:10, Jan Lahoda wrote: Thanks for the comment - done that. Updated webrev: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.03/full/ Delta against the previous iteration: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.03/delta/ H

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-29 Thread Jan Lahoda
On 29.6.2015 12:09, Alan Bateman wrote: On 29/06/2015 10:10, Jan Lahoda wrote: Thanks for the comment - done that. Updated webrev: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.03/full/ Delta against the previous iteration: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.03/delta/ H

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-30 Thread Alan Bateman
On 29/06/2015 12:25, Jan Lahoda wrote: The library is Windows-only, but the WindowsTerminal (or its subclasses) are registered on all platforms using "WindowsTerminal.class". While this does not cause initialization, it seemed safer to ensure the library is only loaded when needed. This see

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-30 Thread Alan Bateman
On 29/06/2015 14:16, Jan Lahoda wrote: I did not realize that - updated webrev: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.04/full/ Delta against previous iteration: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.04/delta/ Feedback is appreciated. This delta change addresses my c

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-30 Thread Jan Lahoda
On 30.6.2015 14:10, Alan Bateman wrote: On 29/06/2015 12:25, Jan Lahoda wrote: The library is Windows-only, but the WindowsTerminal (or its subclasses) are registered on all platforms using "WindowsTerminal.class". While this does not cause initialization, it seemed safer to ensure the library

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-30 Thread Remi Forax
Hi Jay, instead of Callable sup = FLAVORS.get(flavor); if (sup != null) { return sup.call(); } throw new InternalError(); you can write it as a one liner with getOrDefault: FLAVORS.getOrDefault(flavor, () -> { throw new InternalEror(); }).call(); cheers, Rémi On 06/30/2015 03:54 PM, Jan L

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-30 Thread Alan Bateman
On 30/06/2015 14:54, Jan Lahoda wrote: I've changed the registration to use a Callable to produce the Terminal instances instead of registering the Terminal classes: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.05/full/ delta: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.05/delta

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-30 Thread Jan Lahoda
On 30.6.2015 16:27, Alan Bateman wrote: On 30/06/2015 14:54, Jan Lahoda wrote: I've changed the registration to use a Callable to produce the Terminal instances instead of registering the Terminal classes: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.05/full/ delta: http://cr.openjdk.ja

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-07-01 Thread Alan Bateman
On 30/06/2015 16:05, Jan Lahoda wrote: We need to override these "settings" for jshell (we use subclasses of WindowsTerminal/UnixTerminal to get e.g. Ctrl-C detection - not sure if we will want to generalize these additional features for jjs as well). Is there a way to override them (just)

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-07-01 Thread Jan Lahoda
On 1.7.2015 16:43, Alan Bateman wrote: On 30/06/2015 16:05, Jan Lahoda wrote: We need to override these "settings" for jshell (we use subclasses of WindowsTerminal/UnixTerminal to get e.g. Ctrl-C detection - not sure if we will want to generalize these additional features for jjs as well). Is

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-07-02 Thread Jan Lahoda
Hi Ben, (Sorry for a late answer.) Yes, this builds with JDK 9 compiler as part of the JDK build. JLine's own build is uses -source/-target 1.5, which is no longer supported in JDK 9 javac, so this may be one of the issues there. Thanks, Jan On 18.6.2015 22:01, Ben Evans wrote: Hi, Ha

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-07-02 Thread Jan Lahoda
On 1.7.2015 17:58, Jan Lahoda wrote: On 1.7.2015 16:43, Alan Bateman wrote: On 30/06/2015 16:05, Jan Lahoda wrote: We need to override these "settings" for jshell (we use subclasses of WindowsTerminal/UnixTerminal to get e.g. Ctrl-C detection - not sure if we will want to generalize these ad

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-07-04 Thread Ståle W Pedersen
hi, sorry, im a bit late to this discussion, i wasnt aware of any interest for adding jline2 to the jdk until late this week. im working on a project called æsh/aesh: https://github.com/aeshell/aesh its another java console project derived from jline/jline2. it started as an extension/fork of jlin

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-07-07 Thread Jan Lahoda
Hi Ståle, When we were looking for editing libraries, JLine 2 looked best. Unfortunately, we did not find Aesh. At this stage, I think we should proceed with JLine (so the dependent work can continue). Note that it is intended to be JDK internal-only, so there's a possibility to change to a