8169828: jdk/jshell/ExternalEditorTest.java testStatementMush() fails frequently on all platform

8170015: jshell tool: /help output looks terrible on a 100 column wide terminal
8170368: jshell tool: post setting not properly applied, line-ends not prefixed correctly

Reviewed-by: jlahoda

langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java

@@ -279,7 +279,7 @@ public class JShellTool implements MessageHandler {
      */
     @Override
     public void hard(String format, Object... args) {
-        rawout(feedback.getPre() + format + feedback.getPost(), args);
+        rawout(prefix(format), args);
     }
 
     /**
@@ -289,7 +289,7 @@ public class JShellTool implements MessageHandler {
      * @param args printf args
      */
     void error(String format, Object... args) {
-        rawout(feedback.getErrorPre() + format + feedback.getErrorPost(), args);
+        rawout(prefixError(format), args);
     }
 
     /**
@@ -314,18 +314,6 @@ public class JShellTool implements MessageHandler {
         }
     }
 
-    /**
-     * Optional output -- with embedded per- and post-fix
-     *
-     * @param format printf format
-     * @param args printf args
-     */
-    void fluffRaw(String format, Object... args) {
-        if (showFluff()) {
-            rawout(format, args);
-        }
-    }
-
     /**
      * Resource bundle look-up
      *
@@ -351,28 +339,42 @@ public class JShellTool implements MessageHandler {
     }
 
     /**
-     * Add prefixing to embedded newlines in a string, leading with the normal
+     * Add normal prefixing/postfixing to embedded newlines in a string,
-     * prefix
+     * bracketing with normal prefix/postfix
      *
      * @param s the string to prefix
+     * @return the pre/post-fixed and bracketed string
      */
     String prefix(String s) {
-        return prefix(s, feedback.getPre());
+         return prefix(s, feedback.getPre(), feedback.getPost());
     }
 
     /**
-     * Add prefixing to embedded newlines in a string
+     * Add error prefixing/postfixing to embedded newlines in a string,
+     * bracketing with error prefix/postfix
      *
      * @param s the string to prefix
-     * @param leading the string to prepend
+     * @return the pre/post-fixed and bracketed string
      */
-    String prefix(String s, String leading) {
+    String prefixError(String s) {
-        if (s == null || s.isEmpty()) {
+         return prefix(s, feedback.getErrorPre(), feedback.getErrorPost());
+    }
+
+    /**
+     * Add prefixing/postfixing to embedded newlines in a string,
+     * bracketing with prefix/postfix
+     *
+     * @param s the string to prefix
+     * @param pre the string to prepend to each line
+     * @param post the string to append to each line (replacing newline)
+     * @return the pre/post-fixed and bracketed string
+     */
+    String prefix(String s, String pre, String post) {
+        if (s == null) {
             return "";
         }
-        return leading
+        String pp = s.replaceAll("\\R", post + pre);
-                + s.substring(0, s.length() - 1).replaceAll("\\R", System.getProperty("line.separator") + feedback.getPre())
+        return pre + pp + post;
-                + s.substring(s.length() - 1, s.length());
     }
 
     /**
@@ -381,8 +383,7 @@ public class JShellTool implements MessageHandler {
      * @param key the resource key
      */
     void hardrb(String key) {
-        String s = prefix(getResourceString(key));
+        hard(getResourceString(key));
-        cmdout.println(s);
     }
 
     /**
@@ -405,7 +406,7 @@ public class JShellTool implements MessageHandler {
      */
     @Override
     public void hardmsg(String key, Object... args) {
-        cmdout.println(prefix(messageFormat(key, args)));
+        hard(messageFormat(key, args));
     }
 
     /**
@@ -418,7 +419,7 @@ public class JShellTool implements MessageHandler {
     @Override
     public void errormsg(String key, Object... args) {
         if (isRunningInteractive()) {
-            cmdout.println(prefix(messageFormat(key, args), feedback.getErrorPre()));
+            rawout(prefixError(messageFormat(key, args)));
         } else {
             startmsg(key, args);
         }
@@ -431,7 +432,7 @@ public class JShellTool implements MessageHandler {
      * @param args
      */
     void startmsg(String key, Object... args) {
-        cmderr.println(prefix(messageFormat(key, args), ""));
+        cmderr.println(messageFormat(key, args));
     }
 
     /**
@@ -452,15 +453,9 @@ public class JShellTool implements MessageHandler {
         Map<String, String> a2b = stream.collect(toMap(a, b,
                 (m1, m2) -> m1,
                 () -> new LinkedHashMap<>()));
-        int aLen = 0;
-        for (String av : a2b.keySet()) {
-            aLen = Math.max(aLen, av.length());
-        }
-        String format = "   %-" + aLen + "s -- %s";
-        String indentedNewLine = LINE_SEP + feedback.getPre()
-                + String.format("   %-" + (aLen + 4) + "s", "");
         for (Entry<String, String> e : a2b.entrySet()) {
-            hard(format, e.getKey(), e.getValue().replaceAll("\n", indentedNewLine));
+            hard("%s", e.getKey());
+            rawout(prefix(e.getValue(), feedback.getPre() + "\t", feedback.getPost()));
         }
     }
 
@@ -1697,7 +1692,7 @@ public class JShellTool implements MessageHandler {
         } else if (start.isEmpty()) {
             stset = cmd + "-none";
         } else {
-            stset = prefix("startup.jsh:\n" + start + "\n" + cmd + "startup.jsh", "");
+            stset = "startup.jsh:\n" + start + "\n" + cmd + "startup.jsh";
         }
         hard(stset);
     }

langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties

@@ -25,7 +25,7 @@
 
 jshell.msg.welcome =\
 Welcome to JShell -- Version {0}\n\
-For an introduction type: /help intro\n
+For an introduction type: /help intro
 jshell.err.opt.arg = Argument to {0} missing.
 jshell.err.opt.invalid = Invalid options: {0}.
 jshell.err.opt.one = Only one {0} option may be used.
@@ -94,7 +94,8 @@ jshell.msg.help.begin =\
 Type a Java language expression, statement, or declaration.\n\
 Or type one of the following commands:\n
 jshell.msg.help.subject =\n\
-For more information type ''/help'' followed by the name of command or a subject.\n\
+For more information type ''/help'' followed by the name of a\n\
+command or a subject.\n\
 For example ''/help /list'' or ''/help intro''.  Subjects:\n
 
 jshell.err.drop.arg =\
@@ -674,7 +675,7 @@ Show the normal prompt and the continuation-prompts:\n\
 \n\
 Where <mode> is the name of a previously defined feedback mode.\n\
 Where <prompt> and <continuation-prompt> are quoted strings printed as input prompts;\n\
-Both may optionally contain '%s' which will be substituted with the next snippet id --\n\
+Both may optionally contain '%%s' which will be substituted with the next snippet id --\n\
 note that what is entered may not be assigned that id, for example it may be an error or command.\n\
 The continuation-prompt is used on the second and subsequent lines of a multi-line snippet.\n\
 \n\

langtools/test/jdk/jshell/EditorTestBase.java

@@ -253,7 +253,11 @@ public abstract class EditorTestBase extends ReplToolTesting {
                 a -> assertEditOutput(a, "/ed", "b ==> 10", () -> {
                     writeSource(getSource() + "\nint b = 10");
                     exit();
-                })
+                }),
+
+                //TODO: this is a work-around to JDK-8170369
+                a -> assertCommand(a, "1234",
+                        null, "", null, null, "")
         );
     }
 

langtools/test/jdk/jshell/ExternalEditorTest.java

@@ -24,7 +24,7 @@
 /*
  * @test
  * @summary Testing external editor.
- * @bug 8143955 8080843 8163816 8143006
+ * @bug 8143955 8080843 8163816 8143006 8169828
  * @modules jdk.jshell/jdk.internal.jshell.tool
  * @build ReplToolTesting CustomEditor EditorTestBase
  * @run testng ExternalEditorTest

langtools/test/jdk/jshell/ToolCommandOptionTest.java

@@ -23,12 +23,16 @@
 
  /*
  * @test
- * @bug 8157395 8157393 8157517 8158738 8167128 8163840 8167637
+ * @bug 8157395 8157393 8157517 8158738 8167128 8163840 8167637 8170368
  * @summary Tests of jshell comand options, and undoing operations
  * @modules jdk.jshell/jdk.internal.jshell.tool
+ *          jdk.compiler/com.sun.tools.javac.api
+ *          jdk.compiler/com.sun.tools.javac.main
+ * @library /tools/lib
  * @build ToolCommandOptionTest ReplToolTesting
  * @run testng ToolCommandOptionTest
  */
+import java.nio.file.Path;
 import org.testng.annotations.Test;
 import static org.testng.Assert.assertFalse;
 
@@ -240,6 +244,9 @@ public class ToolCommandOptionTest extends ReplToolTesting {
     }
 
     public void setStartTest() {
+        Compiler compiler = new Compiler();
+        Path startup = compiler.getPath("StartTest/startup.txt");
+        compiler.writeToFile(startup, "int iAmHere = 1234;");
         test(
                 (a) -> assertCommand(a, "/set start -furball",
                         "|  Unknown option: -furball -- /set start -furball"),
@@ -257,6 +264,13 @@ public class ToolCommandOptionTest extends ReplToolTesting {
                         ""),
                 (a) -> assertCommand(a, "/set start",
                         "|  /set start -default"),
+                (a) -> assertCommand(a, "/set start " + startup.toString(),
+                        ""),
+                (a) -> assertCommand(a, "/set start",
+                        "|  startup.jsh:\n" +
+                        "|  int iAmHere = 1234;\n" +
+                        "|  \n" +
+                        "|  /set start startup.jsh"),
                 (a) -> assertCommand(a, "/se sta -no",
                         ""),
                 (a) -> assertCommand(a, "/set start",
@@ -265,6 +279,9 @@ public class ToolCommandOptionTest extends ReplToolTesting {
     }
 
     public void retainStartTest() {
+        Compiler compiler = new Compiler();
+        Path startup = compiler.getPath("StartTest/startup.txt");
+        compiler.writeToFile(startup, "int iAmHere = 1234;");
         test(
                 (a) -> assertCommand(a, "/set start -retain -furball",
                         "|  Unknown option: -furball -- /set start -retain -furball"),
@@ -292,7 +309,14 @@ public class ToolCommandOptionTest extends ReplToolTesting {
                 (a) -> assertCommand(a, "/se st -ret",
                         ""),
                 (a) -> assertCommand(a, "/se sta",
-                        "|  /set start -retain -none")
+                        "|  /set start -retain -none"),
+                (a) -> assertCommand(a, "/set start -retain " + startup.toString(),
+                        ""),
+                (a) -> assertCommand(a, "/set start",
+                        "|  startup.jsh:\n" +
+                        "|  int iAmHere = 1234;\n" +
+                        "|  \n" +
+                        "|  /set start -retain startup.jsh")
         );
     }
 

langtools/test/jdk/jshell/ToolSimpleTest.java

@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8153716 8143955 8151754 8150382 8153920 8156910 8131024 8160089 8153897 8167128 8154513
+ * @bug 8153716 8143955 8151754 8150382 8153920 8156910 8131024 8160089 8153897 8167128 8154513 8170015 8170368
  * @summary Simple jshell tool tests
  * @modules jdk.compiler/com.sun.tools.javac.api
  *          jdk.compiler/com.sun.tools.javac.main
@@ -280,6 +280,16 @@ public class ToolSimpleTest extends ReplToolTesting {
         );
     }
 
+    public void testApplicationOfPost() {
+        test(
+                (a) -> assertCommand(a, "/set mode t normal -command", "|  Created new feedback mode: t"),
+                (a) -> assertCommand(a, "/set feedback t", "|  Feedback mode: t"),
+                (a) -> assertCommand(a, "/set format t post \"$%n\"", ""),
+                (a) -> assertCommand(a, "/set prompt t \"+\" \"-\"", ""),
+                (a) -> assertCommand(a, "/set prompt t", "|  /set prompt t \"+\" \"-\"$")
+        );
+    }
+
     public void testHelpLength() {
         Consumer<String> testOutput = (s) -> {
             List<String> ss = Stream.of(s.split("\n"))
@@ -300,14 +310,35 @@ public class ToolSimpleTest extends ReplToolTesting {
                 (a) -> assertHelp(a, "/help", "/list", "/help", "/exit", "intro"),
                 (a) -> assertHelp(a, "/help short", "shortcuts", "<tab>"),
                 (a) -> assertHelp(a, "/? /li", "/list -all", "snippets"),
+                (a) -> assertHelp(a, "/help /set prompt", "optionally contain '%s'", "quoted"),
                 (a) -> assertHelp(a, "/help /help", "/help <command>")
         );
     }
 
+    public void testHelpFormat() {
+        test(
+                (a) -> assertCommandCheckOutput(a, "/help", s -> {
+                    String[] lines = s.split("\\R");
+                    assertTrue(lines.length > 20,
+                            "Too few lines of /help output: " + lines.length
+                          + "\n" + s);
+                    for (int i = 0; i < lines.length; ++i) {
+                        String l = lines[i];
+                        assertTrue(l.startsWith("| "),
+                                "Expected /help line to start with | :\n" + l);
+                        assertTrue(l.length() <= 80,
+                                "/help line too long: " + l.length() + "\n" + l);
+                    }
+                 })
+        );
+    }
+
     private void assertHelp(boolean a, String command, String... find) {
         assertCommandCheckOutput(a, command, s -> {
             for (String f : find) {
-                assertTrue(s.contains(f), "Expected output of " + command + " to contain: " + f);
+                assertTrue(s.contains(f),
+                        "Expected output of " + command + " to contain: " + f
+                      + "\n" + s);
             }
         });
     }