RFR: JDK-8214491: Upgrade to JLine 3.9.0

Sundararajan Athijegannathan sundararajan.athijegannathan at oracle.com
Mon Dec 10 13:52:08 UTC 2018


Thanks for fixing! Interactive mode manual testing is fine - tab 
completion, multi-line edit, built-in/external editor, history object 
all seem fine.

+1

Thanks,
-Sundar

On 10/12/18, 6:33 PM, Jan Lahoda wrote:
> Thanks for testing Sundar!
>
> I've tried to update the jjs completion handling to fix this problem 
> here:
> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.05/
>
> Delta from previous patch:
> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.delta.04.05/
>
> What do you think?
>
> Thanks,
>     Jan
>
> On 10.12.2018 11:14, Sundararajan Athijegannathan wrote:
>> Hi Jan,
>>
>> Tests are fine. Because there are not many UI automated tests for jjs, I
>> tried to manually test few features.
>>
>> Tab-completion seems to be messed up. Only top-level completion seems to
>> work. Property/method completion and inside multi-line method etc. don't
>> seem to work.
>>
>> Thanks,
>> -Sundar
>>
>>
>> On 10/12/18, 3:09 PM, Jan Lahoda wrote:
>>> Hi Sundar,
>>>
>>> Thanks for finding the problem! I was running tier1-3 tests, but seems
>>> these are not there.
>>>
>>> Seems the problem is how JLine(3) handles "dumb" terminals, it
>>> probably does not affect interactive use. So, adjusting the expected
>>> output should hopefully be OK.
>>>
>>> Updated patch:
>>> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.04/index.html
>>> Delta from previous:
>>> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.delta.03.04/
>>>
>>> How does this look?
>>>
>>> Thanks!
>>>
>>> Jan
>>>
>>> On 10.12.2018 06:20, Sundararajan Athijegannathan wrote:
>>>> I built jdk and ran nashorn tests with this patch. Two tests fail with
>>>> this patch - but those tests pass without this patch:
>>>>
>>>>     [testng] Test test/nashorn/script/nosecurity/JDK-8055034.js failed
>>>> at line 1 -
>>>>     [testng]   expected: 'jjs> var x = Object.create(null);'
>>>>     [testng]      found: 'jjs> var x = Object.create(null)jjs> jjs>
>>>> print('PASSED')PASSED'
>>>>
>>>>     [testng] Test test/nashorn/script/nosecurity/JDK-8130127.js failed
>>>> at line 5 -
>>>>     [testng]   expected: 'jjs> print('hello')'
>>>>     [testng]      found: 'jjs> print('hello')hello'
>>>>
>>>> Both tests involving exec'ing jjs and comparing with the expected
>>>> output. Either we need to add these to failing tests and/or address 
>>>> the
>>>> issue before push.
>>>>
>>>> Thanks,
>>>> -Sundar
>>>>
>>>> On 10/12/18, 4:15 AM, Jan Lahoda wrote:
>>>>> On 9.12.2018 16:40, Robert Field wrote:
>>>>>> Ok. Go ahead and incorporate the text.  I'll make a separate bug to
>>>>>> sync
>>>>>> the representation of keys in the other help entries.
>>>>>
>>>>> An updated patch (with your text):
>>>>> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.03/
>>>>> Delta from previous patch:
>>>>> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.delta.02.03/
>>>>>
>>>>> Thanks for the comments and for the text! Any feedback is welcome.
>>>>>
>>>>> Thanks,
>>>>>     Jan
>>>>>
>>>>>
>>>>>>
>>>>>> Robert
>>>>>>
>>>>>>
>>>>>> On December 9, 2018 6:36:26 AM Jan Lahoda <jan.lahoda at oracle.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Hi Robert,
>>>>>>>
>>>>>>> On 9.12.2018 02:40, Robert Field wrote:
>>>>>>>> OK, here is my thinking on a /help keys entry.  Note: I'm using 
>>>>>>>> the
>>>>>>>
>>>>>>> I think this looks great!
>>>>>>>
>>>>>>>> representation for keys used by the User's Guide -- which I
>>>>>>>> assume is
>>>>>>>> the official tech pubs style -- seems we should be consistent; If
>>>>>>>> we are
>>>>>>>> however, the other /help entries should be brought into compliance
>>>>>>>> (they
>>>>>>>> seem to be all over the map already).  May make sense to spin
>>>>>>>> this off
>>>>>>>> as a separate bug, which I'd be happy to take on.
>>>>>>>
>>>>>>> I'm happy with a separate bug, if you prefer/are OK with that. Or
>>>>>>> I can
>>>>>>> incorporate the text into upgrade patch, whichever you prefer.
>>>>>>>
>>>>>>> I've updated my current patch with only a rename "editing" -> 
>>>>>>> "keys"
>>>>>>> and
>>>>>>> the fix to the jshell tool name:
>>>>>>> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.02/
>>>>>>>
>>>>>>> Delta from last:
>>>>>>> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.delta.01.02/
>>>>>>>
>>>>>>> Jan
>>>>>>>
>>>>>>>>
>>>>>>>> Comments welcome ---
>>>>>>>>
>>>>>>>> __________________________________________________________________________________________________________________________ 
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> /help keys
>>>>>>>> ==========
>>>>>>>>
>>>>>>>> The jshell tool provides line editing support to allow you to
>>>>>>>> navigate
>>>>>>>> within and edit snippets and commands. The current
>>>>>>>> command/snippet can
>>>>>>>> be edited, or prior commands/snippets can be retrieved from 
>>>>>>>> history,
>>>>>>>> edited, and executed. This support is similar to readline/editline
>>>>>>>> with
>>>>>>>> simple emacs-like bindings.  There are also jshell tool 
>>>>>>>> specific key
>>>>>>>> sequences.
>>>>>>>>
>>>>>>>> --- Line and history navigation ---
>>>>>>>>
>>>>>>>> Return
>>>>>>>>    Enters the current snippet
>>>>>>>> Left-arrow or Ctrl+B
>>>>>>>>    Moves backward one character
>>>>>>>> Right-arrow or Ctrl+F
>>>>>>>>    Moves forward one character
>>>>>>>> Up-arrow or Ctrl+P
>>>>>>>>    Moves up one line, backward through history
>>>>>>>> Down arrow or Ctrl+N
>>>>>>>>    Moves down one line, forward through history
>>>>>>>> Ctrl+A
>>>>>>>>    Moves to the beginning of the line
>>>>>>>> Ctrl+E
>>>>>>>>    Moves to the end of the line
>>>>>>>> Meta+B
>>>>>>>>    Moves backward one word
>>>>>>>> Meta+F
>>>>>>>>    Moves forward one word
>>>>>>>> Ctrl+R
>>>>>>>>    Search backward through history
>>>>>>>>
>>>>>>>>
>>>>>>>> --- Line and history basic editing ---
>>>>>>>>
>>>>>>>> Meta+Return or Ctrl+Return (depending on platform)
>>>>>>>>    Insert a new line in snippet
>>>>>>>> Ctrl+_ (underscore may require shift key) or Ctrl-X then Ctrl-U
>>>>>>>>    Undo edit - repeat to undo more edits
>>>>>>>> Delete
>>>>>>>>    Deletes the character at or after the cursor, depending on the
>>>>>>>> operating system
>>>>>>>> Backspace
>>>>>>>>    Deletes the character before the cursor
>>>>>>>> Ctrl+K
>>>>>>>>    Deletes the text from the cursor to the end of the line
>>>>>>>> Meta+D
>>>>>>>>    Deletes the text from the cursor to the end of the word
>>>>>>>> Ctrl+W
>>>>>>>>    Deletes the text from the cursor to the previous white space
>>>>>>>> Ctrl+Y
>>>>>>>>    Pastes the most recently deleted text into the line
>>>>>>>> Meta+Y
>>>>>>>>    After Ctrl+Y, Meta+Y cycles through previously deleted text
>>>>>>>> Ctrl+X then Ctrl+K
>>>>>>>>    Delete whole snippet
>>>>>>>>
>>>>>>>>
>>>>>>>> --- Shortcuts for jshell tool ---
>>>>>>>>
>>>>>>>> For details, see: /help shortcuts
>>>>>>>>
>>>>>>>> Tab
>>>>>>>>    Complete Java identifier or jshell command
>>>>>>>> Shift+Tab then v
>>>>>>>>    Convert expression to variable declaration
>>>>>>>> Shift+Tab then m
>>>>>>>>    Convert statement to method declaration
>>>>>>>> Shift+Tab then i
>>>>>>>>    Add imports for this identifier
>>>>>>>>
>>>>>>>>
>>>>>>>> --- More line and history editing ---
>>>>>>>>
>>>>>>>> Ctrl+L
>>>>>>>>    Clear screen and reprint snippet
>>>>>>>> Ctrl+U
>>>>>>>>    Kill whole line
>>>>>>>> Ctrl+T
>>>>>>>>    Transpose characters
>>>>>>>> Ctrl+X then Ctrl+B
>>>>>>>>    Navigate to matching bracket, parenthesis, ...
>>>>>>>> Ctrl+X then =
>>>>>>>>    Enter show current character position mode
>>>>>>>> Ctrl+X then Ctrl+O
>>>>>>>>    Toggle overwrite characters vs insert characters
>>>>>>>> Meta+C
>>>>>>>>    Capitalize word
>>>>>>>> Meta+U
>>>>>>>>    Convert word to uppercase
>>>>>>>> Meta+L
>>>>>>>>    Convert word to lowercase
>>>>>>>> Meta+0 through Meta+9 then key
>>>>>>>>    Repeat the specified number of times
>>>>>>>>
>>>>>>>> Where, for example, "Ctrl+A" means hold down the control key and
>>>>>>>> press A.
>>>>>>>> Where "Meta" is "Alt" on many keyboards.
>>>>>>>> Line editing support is derived from JLine 3.
>>>>>>>>
>>>>>>>> _______________________________________________________________________________ 
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> -Robert
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 12/8/18 11:47 AM, Jan Lahoda wrote:
>>>>>>>>> On 8.12.2018 20:39, Robert Field wrote:
>>>>>>>>>> Thanks for updating.
>>>>>>>>>>
>>>>>>>>>> Good to have /help for line editing keys, however:
>>>>>>>>>>    (1) The supported keys should be listed without requiring 
>>>>>>>>>> that
>>>>>>>>>> the
>>>>>>>>>> user knows or looks-up readline. More so, since only some 
>>>>>>>>>> readline
>>>>>>>>>> keys
>>>>>>>>>> are supposed.
>>>>>>>>>>    (2) "/edit" is a command, so with this change, typing "/help
>>>>>>>>>> edit"
>>>>>>>>>> would give both the /edit and editing help -- which would be
>>>>>>>>>> confusing.
>>>>>>>>>> I suggest "/help keys", there are no "k" help items.
>>>>>>>>>
>>>>>>>>> Will do.
>>>>>>>>>
>>>>>>>>>>    (3) In jshell tool online docs the tool is spelled "jshell
>>>>>>>>>> tool"
>>>>>>>>>> I'd be happy to write up suggested text.
>>>>>>>>>
>>>>>>>>> Thanks, that would be very welcome!
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> As to the alternative patch: I understand the motivation, it
>>>>>>>>>> would be
>>>>>>>>>> nice to just use the natural "Enter" as new line. Unfortunately
>>>>>>>>>> it's
>>>>>>>>>> functionality is then overloaded.  I believe the alternative
>>>>>>>>>> will be
>>>>>>>>>> confusing and awkward:
>>>>>>>>>>    (1) It breaks backward compatibility in user experience, 
>>>>>>>>>> Enter
>>>>>>>>>> has
>>>>>>>>>> always been accept/evaluate
>>>>>>>>>>    (2) It would be very awkward and non-intuitive (particularly
>>>>>>>>>> in a
>>>>>>>>>> long multiline snippet) to have to navigate to the end of the
>>>>>>>>>> snippet to
>>>>>>>>>> accept.
>>>>>>>>>>         Note: must be the end both vertically and horizontally.
>>>>>>>>>>    (3) It is inconsistent: Mid snippet Enter on one line does
>>>>>>>>>> accept.
>>>>>>>>>> However, on a continuation line (say after "int x =") it adds a
>>>>>>>>>> new
>>>>>>>>>> line
>>>>>>>>>> instead.
>>>>>>>>>>    (4) It doesn't solve the problem for the most common
>>>>>>>>>> location for
>>>>>>>>>> adding a new line, the end of snippet -- thus introducing more
>>>>>>>>>> inconsistency.
>>>>>>>>>
>>>>>>>>> Ok. This was meant more as a backup in case there's no reasonable
>>>>>>>>> shortcut we could use to add new lines.
>>>>>>>>>
>>>>>>>>> Thanks for the comments!
>>>>>>>>>
>>>>>>>>> Jan
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Robert
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 12/8/18 7:21 AM, Jan Lahoda wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> I've updated the patch to reflect the comments so far, the
>>>>>>>>>>> current
>>>>>>>>>>> version is here:
>>>>>>>>>>> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.01/
>>>>>>>>>>> Delta since the previous version is here for convenience:
>>>>>>>>>>> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.delta.00.01/
>>>>>>>>>>>
>>>>>>>>>>> In this version, while editing a snippet, a new line can be 
>>>>>>>>>>> added
>>>>>>>>>>> using Alt-Enter (on platforms that support that), or
>>>>>>>>>>> Ctrl-Enter (on
>>>>>>>>>>> Windows).
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> And alternative patch is here:
>>>>>>>>>>> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.01a/
>>>>>>>>>>> Delta:
>>>>>>>>>>> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.delta.00.01a/ 
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> In the alternative version, Enter will only "confirm" 
>>>>>>>>>>> multi-line
>>>>>>>>>>> snippet when the cursor is at the end of the snippet,
>>>>>>>>>>> otherwise it
>>>>>>>>>>> will add a new line (so that a special shortcut is not needed;
>>>>>>>>>>> but a
>>>>>>>>>>> snippet cannot be "confirmed" by pressing Enter in the middle
>>>>>>>>>>> of a
>>>>>>>>>>> multi-line snippet.)
>>>>>>>>>>>
>>>>>>>>>>> The only difference between the .01 and .01a patches is the way
>>>>>>>>>>> they
>>>>>>>>>>> allow to add new lines into the multi-line snippets. Other
>>>>>>>>>>> changes
>>>>>>>>>>> are
>>>>>>>>>>> the same.
>>>>>>>>>>>
>>>>>>>>>>> Any feedback on this is welcome.
>>>>>>>>>>>
>>>>>>>>>>> Thanks!
>>>>>>>>>>>
>>>>>>>>>>> Jan
>>>>>>>>>>>
>>>>>>>>>>> On 5.12.2018 18:18, Jan Lahoda wrote:
>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>
>>>>>>>>>>>> On 4.12.2018 23:59, Robert Field wrote:
>>>>>>>>>>>>> I saw no issues with JShell tool and test portions of the
>>>>>>>>>>>>> webrev.  I
>>>>>>>>>>>>> did
>>>>>>>>>>>>> not review the nashorn changes.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks for looking at this!
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Testing it: editing multi-line snippets is vastly easier and
>>>>>>>>>>>>> more
>>>>>>>>>>>>> intuitive.
>>>>>>>>>>>>> There is one issue, with the old mechanism, as horridly 
>>>>>>>>>>>>> clunky
>>>>>>>>>>>>> as it
>>>>>>>>>>>>> was, you could add new lines of code (a frequently needed
>>>>>>>>>>>>> functionality).
>>>>>>>>>>>>> Since <return> is accept, I could find no way to add lines 
>>>>>>>>>>>>> with
>>>>>>>>>>>>> this
>>>>>>>>>>>>> JLine 3 version.  Thoughts?
>>>>>>>>>>>>
>>>>>>>>>>>> One possibility that comes to mind: pressing Enter inside the
>>>>>>>>>>>> snippet
>>>>>>>>>>>> would add a new line, Enter at the very end of a (complete)
>>>>>>>>>>>> snippet
>>>>>>>>>>>> would confirm that snippet. Could be fairly
>>>>>>>>>>>> convenient/intuitive.
>>>>>>>>>>>> What
>>>>>>>>>>>> do you think?
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I looked into readline commands as an approach to addressing
>>>>>>>>>>>>> this,
>>>>>>>>>>>>> found
>>>>>>>>>>>>> nothing.  However, most readline commands worked. Ctrl-u
>>>>>>>>>>>>> however
>>>>>>>>>>>>> did
>>>>>>>>>>>>> not
>>>>>>>>>>>>> behave as documented in readline (instead deleting to the
>>>>>>>>>>>>> beginning of
>>>>>>>>>>>>> the line).  I notice that the there is zero in-command
>>>>>>>>>>>>> documentation of
>>>>>>>>>>>>> command-line editing -- not even a mention.  Independent
>>>>>>>>>>>>> from the
>>>>>>>>>>>>> review
>>>>>>>>>>>>> of this port, it seems we should have in-command
>>>>>>>>>>>>> documentation of
>>>>>>>>>>>>> command editing -- even more so now that multiline editing is
>>>>>>>>>>>>> useful.
>>>>>>>>>>>>
>>>>>>>>>>>> This is about enhancing /help, right? I'll see what I can do.
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> The JShell User Manual will also need a little edit in the
>>>>>>>>>>>>> History
>>>>>>>>>>>>> Navigation section.
>>>>>>>>>>>>
>>>>>>>>>>>> Where is that?
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>      Jan
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> -Robert
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 11/29/18 1:06 PM, Jan Lahoda wrote:
>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I'd like to update the internal JLine used by JShell and
>>>>>>>>>>>>>> jjs to
>>>>>>>>>>>>>> JLine
>>>>>>>>>>>>>> 3.9.0. Two notable advantages of this version is multi-line
>>>>>>>>>>>>>> snippet
>>>>>>>>>>>>>> editing and better UI on Windows (escape sequence support on
>>>>>>>>>>>>>> Windows).
>>>>>>>>>>>>>> As a consequence, this patch drops EditingHistory, as it 
>>>>>>>>>>>>>> does
>>>>>>>>>>>>>> not
>>>>>>>>>>>>>> seem
>>>>>>>>>>>>>> to be needed anymore.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> JBS:
>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8214491
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The full patch is here:
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.00/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> To make the changes more clear, I've split it into two:
>>>>>>>>>>>>>> -replacement of existing JLine with the new on in
>>>>>>>>>>>>>> jdk.internal.le, no
>>>>>>>>>>>>>> changes to JLine code:
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.00.replace/ 
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> -tweaks to JLine (repackaging, eliminating references to
>>>>>>>>>>>>>> j.u.l.Logger,
>>>>>>>>>>>>>> adding hooks to wrap input streams with our stop-detecting
>>>>>>>>>>>>>> input
>>>>>>>>>>>>>> stream, adding unicode escapes to unicode characters, 
>>>>>>>>>>>>>> support
>>>>>>>>>>>>>> for
>>>>>>>>>>>>>> Windows without JNA), adjustments to JShell and jjs
>>>>>>>>>>>>>> (unfortunately,
>>>>>>>>>>>>>> 3.9.0 is not compatible with the JLine 2 branch, so the
>>>>>>>>>>>>>> changes
>>>>>>>>>>>>>> are
>>>>>>>>>>>>>> substantial):
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jlahoda/8214491/webrev.00.update/ 
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Any feedback is welcome!
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>     Jan
>>>>>>
>>>>>>
>>>>>>


More information about the nashorn-dev mailing list