RFR 8146138 et. al.: jshell tool: add /help command plus misc fixes

Robert Field robert.field at oracle.com
Wed Feb 10 01:17:59 UTC 2016


New webrev:
     http://cr.openjdk.java.net/~rfield/8146138v2.webrev/

See below...

On 02/09/16 04:27, Jan Lahoda wrote:
> Hi Robert,
>
> Overall, seems OK to me.
>
> Comments:
> -in the external editor handling, do we need to care about editor 
> commands that have a space in the path? (I.e. is there a need for the 
> user to quote the external editor path?)

Absolutely true, new webrev allows quoting of command and parameters; 
Also, does argument splitting in a cleaner way.

> -I am not quite sure which part of the patch relates to JDK-8147898 
> (/reload quiet)?

It already works (fixed not sure when), all it was missing was a test, 
which this adds.

Note: the new webrev includes tests for 8147886 and 8147887.

> -in JShellTool.cmdHelp(), when the full help is constructed for 
> commands, it always ends with "\n\n", which then produces several 
> empty lines at the end of the help when printed. Would it make sense 
> to strip some of the trailing newlines so that there would be only a 
> single empty line between the help and the prompt?

Yeah, it printed three blank lines -- which is too much.  Tried with 
one, but since help messages are so long, didn't seem enough to see the 
prompt.  Also, starting the command/subject title without blank line 
looks weird.  So, what I've done is rotate one new-line to the top -- 
this keeps a two-blank separator between subjects if there are more than 
one.

> -just a note about future - we might provide a completion for /help as 
> well.

True. Added:
      8149498:jshell tool: command completion on /help <command> and 
<subject>

Thanks Jan!

-Robert

>
> Jan
>
> On 28.1.2016 22:41, Robert Field wrote:
>> Fixes:
>>
>> 8146138 jshell tool: add /help <command>
>> 8147495 jshell tool: correctly handle arguments on /seteditor command
>> 8147886 jshell tool: commands don't allow reference to start-up or
>> explicit id of dropped/failed snippets
>> 8147887 jshell tool: /list start -- fails
>> 8147898 jshell tool: /reload quiet -- should quiet echo
>>
>> Webrev:
>>
>>    http://cr.openjdk.java.net/~rfield/8146138v0.patch/
>>
>> Thanks,
>> Robert
>>



More information about the kulla-dev mailing list