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

Jan Lahoda jan.lahoda at oracle.com
Tue Mar 8 14:55:25 UTC 2016


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

-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"

+     * @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.

?

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


More information about the kulla-dev mailing list