RFR 9: JDK-8177076: jshell tool: usability of completion

[Robert Field]

On 04/04/17 06:27, Jan Lahoda wrote:
> Thanks for the comments. I prepared three patches (see inline 
> comments) for individual parts (so that we can skip some of them for 9 
> if needed), a complete webrev is here:
> http://cr.openjdk.java.net/~jlahoda/8178013/webrev.00/
>
> On 1.4.2017 01:57, John Rose wrote:
>> On Mar 27, 2017, at 1:39 AM, Jan Lahoda <jan.lahoda at oracle.com
>> <mailto:jan.lahoda at oracle.com>> wrote:
>>>
>>> Hello,
>>>
>>> I'd like to ask for a review of a patch that merges shift-tab and tab
>>> completions, and changes the fix shortcut to shift-tab:
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8177076
>>> Webrev: http://cr.openjdk.java.net/~jlahoda/8177076/webrev.00/
>>>
>>> Any feedback is welcome,
>>>     Jan
>>
>> A couple of comments (on webrev.03):
>>
>> This help string has the wrong whitespace:
>>
>> +        a variable declaration whose type is based on the type of the
>> expression.\n\t\t\
>> +Shift-<tab> i\n\t\t\
>>
>> The first \n\t\t\ should be \n\n.
>>
>> Help strings should not mention <fix> anymore:
>>
>> +Invalid <fix> character.  Use "i" for auto-import or "v" for variable
>> creation.  For more information see:\n\
>> +   /help shortcuts
>>
>> I suggest:  Unexpected character after Shift-Tab.  Use "i" …
>
> Webrev for the two above is here:
> http://cr.openjdk.java.net/~jlahoda/8178013/webrev.a.00/

These changes look good.

This help ends (line 507) with two tabs as well.  Help text should end 
with no special character (and yes, very inconsistent), see help.context
So, this should be "...classpath."

Oops, we haven't updated the copyright, can do that now.

Don't need a re-review for these.

This is a noreg-doc if it were a stand-alone bug, and could be pushed 
without approval.  This might be a good idea as we are running out of 
l10n rope.

>
>>
>> I built your patch and tried it out.  I like the new behavior very much.
>> The "Shift-Tab i" command is more friendly.
>>
>> Thanks!
>>
>> I noticed a couple of oddities.
>>
>> First of all, "/help /set <Tab>" prints the "/set" subcommands, but
>> "/help set <Tab>" does not.
>>
>> jshell> /help /set <Tab>
>> editor       feedback     format       mode         prompt start
>>        truncation
>> jshell> /help set <Tab>
>> get information about jshell   <==== YES, I THOUGHT I WAS DOING THAT?
>
> Webrev for this is here:
> http://cr.openjdk.java.net/~jlahoda/8178013/webrev.b.00/

Looks good.

This will need approval (of course) but it is very simple, and that 
should help.  Or we wait for an update release an save the hassle.

Oh, my, we haven't updated copyright on this either.


>
>>
>>
>> Second, when completing inside help subcommands, the "todo" list is
>> packed with generic
>> "help on help", which promises information but isn't that useful.  If
>> you are already saying
>> "/help intro <Tab>" it doesn't seem very helpful to back up to the same
>> help text as
>> "/help <Tab><Tab>".
>>
>> A further example of the odd help-doc is below.
>
> Webrev for this is here:
> http://cr.openjdk.java.net/~jlahoda/8178013/webrev.c.00/index.html

1708: This should also work if the prefix were "/?" or "/he"...
1709: This loses the cursor info
1716: Since commands includes hidden and pseudo commands they will show 
up here.
And since commands includes subjects (intended here) I believe that the 
comparison starting with the second letter will include help starting 
with the letters of "ontext", "ntro", and "hortcut"

This is complex enough and solves a small enough problem that I can't 
see trying to approve it for JDK 9.
We pressing 9 issues, so let's save our strength.

Thanks,
Robert

>
> Any comments are welcome.
>
> Thanks,
>     Jan
>
>>
>> — John
>>
>> jshell> /help /set tr<Tab>
>> ===REDRAW===>
>> jshell> /help /set truncation<Tab>
>> <press tab again to see synopsis>
>>
>> jshell> /help /set truncation<Tab>
>> get information about jshell   <==== NOT A SYNOPSIS OF "truncation"
>>
>> <press tab again to see full documentation>
>>
>> jshell> /help /set truncation<Tab>
>> Display information about jshell.   <==== NOT FULL DOC OF "truncation"
>> /help
>>      List the jshell commands and help subjects.
>>
>> /help <command>
>>      Display information about the specified command. The slash must be
>> included.
>>      Only the first few letters of the command are needed -- if more
>> than one
>>      each will be displayed.  Example:  /help /li
>>
>> /help <subject>
>>      Display information about the specified help subject. Example:
>> /help intro
>>
>> jshell>
>>