RFR 9: JDK-8177076: jshell tool: usability of completion
Robert Field
robert.field at oracle.com
Tue Mar 28 19:31:19 UTC 2017
On 03/28/17 09:56, Jan Lahoda wrote:
> Hi Robert,
>
> An updated version of the patch:
> http://cr.openjdk.java.net/~jlahoda/8177076/webrev.01/
I like the new CompletionTask design!
ConsoleIOContext.java
----------------------------
Copyright need update.
l10n.properties
------------------
156 jshell.console.see.next.page = <press space for next page, any other
key to quit>
tab also works, so should be "tab or space"
157 jshell.console.see.next.javadoc = <press tab or space for next
javadoc, any other key to quit>
If this remains (see below suggestion to change this), then it should be
"documentation" rather than "javadoc", as elsewhere.
493 If there is more than one completion, show possible
completions.\n\t\t\
" then possible completions will be shown"
496 After a complete expression, press "Shift-<tab> v" to
introduce a new variable\n\t\t\
It may not be clear to people what those keyboard actions are
"After a complete expression, hold down <shift> while pressing <tab>,
then release and press "v", the expression will be converted to a
variable declaration"
498 Shift-<tab> i\n\t\t\
Same treatment. And maybe "propose possible imports which will resolve
the name..."
MergedTabShiftTabTest.java
---------------------------------
No bug ID on test.
User Interface
------------------
I'm wondering, would it be more consistent, when displaying javadoc, if
instead of displaying this prompt between pages:
<press tab or space for next javadoc, any other key to quit>
That it did a redrawLine()?
<press tab again for next documentation>
jshell> l.add(
Implementation-wise, that each doc page was an item in the the todo list.
Basically, I think getting rid of all prompts would be cleanest.
-------
What a tease, tell me there is documentation, and then... ;-)
jshell> l
l long
Signatures:
l:java.util.List
<press tab again to see documentation>
jshell> l
l:java.util.List
<no javadoc found>
jshell> l
-------
What to do when there are no completions? Like:
jshell> /zz<tab>
or
jshell> zz<tab>
Before it did nothing, which is confusing.
It could print "No completions" and redraw, but that would probably be
noisy.
I think what it does with this patch, which is redraw right below, is
probably just right for snippets.
jshell> /zz<tab>
jshell> /zz
Though, for commands (but not command arguments), maybe "No such
command" and a redraw???
-------
shift-tab for fixes sure is nice.
--------------
Nice work!
-Robert
>
> On 27.3.2017 23:20, Robert Field wrote:
>> ConsoleIOContext.java
>> ----------------------------
>>
>> One of the key pieces of usability feedback was to step away from modal
>> prompts.
>> Where, the cycle is on-tab: next-info/redraw.
>> These changes move to that mechanism.
>> However, the "too many completions" case uses a modal prompt (556).
>> Seems this could be handled the same way.
>
> Not sure if there's a modal prompt on ConsoleIOContext:556, rev.00 -
> there is a single read and after that the execution continues one way
> or another? It is true the code looked a little bit different that in
> the other cases. Anyway, this code does not exist in rev.01.
>
>>
>> Nit: the variable declared on line 211: "String test", is that intended
>> to be "text"?
>
> Yes, thanks. Fixed.
>
>>
>> SourceCodeAnalysisImpl.java
>> ------------------------------------
>>
>> These changes appear to be incomplete or abandoned. They introduce a
>> variable that is never set.
>
> Used by tests. Comment added.
>
>>
>> User Interface
>> ------------------
>>
>> jshell> List li
>> l ==> null
>>
>> jshell> li.<tab>
>> add( addAll( clear() contains(
>> containsAll( equals( forEach( get( getClass()
>> hashCode() indexOf( isEmpty() iterator()
>> lastIndexOf( listIterator( notify() notifyAll()
>> parallelStream()
>> remove( removeAll( removeIf( replaceAll(
>> retainAll( set( size() sort( spliterator()
>> stream() subList( toArray( toString() wait(
>>
>> jshell> li.add<tab>
>> add( addAll(
>>
>> jshell> li.add(<tab>
>> Current signatures:
>> boolean List<E extends Object>.add(E e)
>> void List<E extends Object>.add(int index, E element)
>>
>> Possible completions:
>>
>> l xxx xyz
>>
>> <press tab again to see documentation>
>>
>> jshell> li.add(
>>
>> The first and second tab used in the example above give possible
>> completions without a leading header "Possible completions:" of the
>> third case. This should be consistent.
>> The header could be added everywhere, but I think that would be noisy.
>> The change in ordering is confusing anyhow.
>> The prompt "<press tab again to see documentation>" would be more
>> intuitive grouped with the method signatures.
>> The header "Current signatures:", I think would be better
>> "Signatures:", "Method signatures:", or "Signatures of add:".
>> Applying these:
>>
>> jshell> li.add(<tab>
>> l xxx xyz
>>
>> Method signatures:
>> boolean List<E extends Object>.add(E e)
>> void List<E extends Object>.add(int index, E element)
>>
>> <press tab again to see documentation>
>
> Done. (The caption is "Signatures:", as in general, it is not always
> method signatures there.)
>
>>
>> jshell> li.add(
>>
>> See above, under ConsoleIOContext.java, additionally, the inlining of
>> more text makes this more confusing, as no indication of what to do if
>> you don't want it --
>
> Sorry, but I am not sure what is the action requested here.
>
>>
>> jshell> 4 + <tab>
>>
>> Gets replaced with:
>>
>> jshell> 4 + 542 possible completions. Press <tab> or y to see them all.
>>
>> I'd opt for consistency (which also uses this prompt that is used in the
>> after documentation case):
>>
>> jshell> 4 + <tab>
>> <press tab again to see all possible completions; total possible
>> completions: 542>
>
> Done. To do that, I re-wrote the support a little (as this basically
> introduces a new state in the state "graph".)
>
>>
>> jshell> 4 +
>>
>> I see also that if documentation is displayed and there are smart items,
>> then the last tab choice is all items. In no other case are all items
>> offered if there are smart items.
>
> I believe all items may be shown where smart items exist and there's
> no documnetation:
> jshell> String str = new Str<tab>
> String(
>
> <press tab again to see all possible completions>
>
> jshell> String str = new Str<tab>
> StreamCorruptedException( StreamTokenizer(
> String( StringBuffer(
> StringBufferInputStream( StringBuilder(
> StringIndexOutOfBoundsException( StringJoiner(
> StringReader( StringTokenizer( StringWriter(
>
> jshell> String str = new Str
>
>>
>> The command help is a very nice feature.
>>
>> I'm unsure of the "<press tab again to see synopsis>" prompt line as the
>> synopsis is also one line.
>
> There may be multiple lines with synopsis, I believe:
> jshell> /e<tab>
> /edit /env /exit
>
> <press tab again to see synopsis>
>
> jshell> /e<tab>
> /edit
> edit a source entry referenced by name or id
>
> /env
> view or change the evaluation context
>
> /exit
> exit jshell
>
> <press tab again to see full documentation>
>
> jshell> /e
>
>>
>> Overall
>> ---------
>>
>> Perhaps it would be good to lay this out as a UI design before turning
>> it into code.
>>
>> Naively (as I haven't done it), I think it may be possible to regularize
>> the processing (starting as pseudo-code derived from the UI design).
>> I note 19 conditionals in the new code. I'm concerned about the
>> branching complexity not only for code maintenance and testing but for
>> what it implies about the user model.
>
> I think the primary question is if it works well for the users. Not
> sure if the # of conditionals is so significant. I didn't really try
> to minimize the # of conditions, I tried to make the code
> understandable, making it simpler to say "if the situation is this,
> then the outcome will be that". (The new revision goes one more step
> in that direction, I think.)
>
> (Having this in the code also helps to cover all the corners, which
> are easy to miss otherwise.)
>
> Anyway - if there's a UI design covering all the cases
> (command/expression, has/hasn't doc, has/hasn't smart items,
> has/hasn't additional items, there are/aren't too many items), I can
> implement it.
>
> Jan
>
>>
>> I will be available to help on this tomorrow, if you would like.
>>
>> -Robert
>>
>>
>> On 03/27/17 01:39, Jan Lahoda 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
>>
More information about the kulla-dev
mailing list