RFR 8148316: jshell tool: Configurable output format, et. al. - v9

[Robert Field]

On 03/08/16 06:55, Jan Lahoda wrote:
> Overall, seems OK to me. Some minor comments:
>
> -should initializeFeedbackModes be moved to Feedback, or to a separate 
> file (that would be automatically loaded on startup)? But maybe it 
> does not consume too much space in JShellTool

I initially did this but it required writing a version of the /set 
command to live in Feedback.  Finally I decided that Feedback is the 
engine and the init was the configuration (which we know will change) so 
it was fine to have them separate.

>
> -Nit in JShellTool.java:
> @@ -183,45 +253,106 @@
>      // Tool id (tid) mapping: the current name spaces
>      NameSpace currentNameSpace;
>
>      Map<Snippet,SnippetInfo> mapSnippet;
>
> +    /**
> +     * I the input/output currently interactive
>
> Typo: "I" -> "Is"

Fixed.

>
> +     * @return true if console
> +     */
> +    boolean interactive() {
> +         return input != null && input.interactiveOutput();
> +    }
> +
>
> -in the /help /set, currently it looks like this:
> |  The contents of the specified <file> become the default start-up 
> snippets and commands.
> |  /set start <file>
>
> should it be in the opposite order, i.e.:
> |  /set start <file>
> |  The contents of the specified <file> become the default start-up 
> snippets and commands.
>
> ?

Everywhere else is command first.  Will reverse.

Fixed.  Also switched to the indent style used in help for other commands --

-> /help /set
|
|  /set
|
|  Set jshell configuration information, including:
|  the external editor to use, the start-up definitions to use, a new 
feedback mode,
|  the command prompt, the feedback mode to use, or the format of output.
|
|  /set editor <command> <optional-arg>...
|    -- Specify the command to launch for the /edit command.
|       The <command> is an operating system dependent string.
|
|  /set start <file>
|    -- The contents of the specified <file> become the default start-up 
snippets and commands.
|
|  /set feedback <mode>
|    -- Set the feedback mode describing displayed feedback for entered 
snippets and commands.
|
|  /set newmode <new-mode> [command|quiet [<old-mode>]]
|    -- Create a user-defined feedback mode, optionally copying from an 
existing mode.
|
|  /set prompt <mode> "<prompt>" "<continuation-prompt>"
|    -- Set the displayed prompts for a given feedback mode.
|
|  /set format <mode> "<format>" <selector>...
|    -- Configure a feedback mode by setting the format to use in a 
specified set of cases.
|
|  /set field 
name|type|result|when|action|resolve|pre|post|errorpre|errorpost 
"<format>"  <format-case>...
|    -- Set the format of a field within the <format-string> of a "/set 
format" command
|
|  To get more information about one of these forms, use /help with the 
form specified.
|  For example:   /help /set format
|


Thanks Jan!

-Robert


>
> Jan
>
> On 4.3.2016 09:04, Robert Field wrote:
>> Update:
>>    Per Brian's review have added setting prompt -- /set prompt
>>    Per Jan's review have nice consistent verbose /help support and error
>> messages, which meant rolling in the /set unification,
>>    have moved feedback support to its own class, added much more
>> extensive testing including invalid input, and addressed the other
>> smaller issues.
>>    Detailed help is available, for example:  /help set format
>>    To achieve clean errors, made new feedback mode creation explicit --
>> /set newmode
>>
>> Bugs (the second (unrelated) bug was discovered and fixed in the process
>> of testing, the third bug was added to address output consistency)
>>      https://bugs.openjdk.java.net/browse/JDK-8148316
>>      https://bugs.openjdk.java.net/browse/JDK-8149524
>>      https://bugs.openjdk.java.net/browse/JDK-8148317
>>
>> Webrev:
>>      http://cr.openjdk.java.net/~rfield/8148316v9.webrev/
>>
>> The example output configuration has been updated accordingly (Open with
>> /open):
>> http://cr.openjdk.java.net/~rfield/formats/arrowResultNlMeta.jsh
>>
>> Thanks,
>> Robert