From 96c44308adbb9b34f7285c329953d22a5d0a6e2f Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Mon, 8 Feb 2021 09:32:40 +0100 Subject: [PATCH 1/7] 8247403: JShell: No custom input (e.g. from GUI) possible with JavaShellToolBuilder --- .../jshell/tool/ConsoleIOContext.java | 14 ++-- .../jdk/jshell/CustomInputToolBuilder.java | 74 +++++++++++++++++++ 2 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 test/langtools/jdk/jshell/CustomInputToolBuilder.java diff --git a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java index 610404b8738c2..d766172a2ebb7 100644 --- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java +++ b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2021, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -116,8 +116,8 @@ public int readBuffered(byte[] b) throws IOException { } }; Terminal terminal; - if (System.getProperty("test.jdk") != null) { - terminal = new TestTerminal(nonBlockingInput, cmdout); + if (cmdin != System.in) { + terminal = new NonSystemInTerminal(nonBlockingInput, cmdout); input.setInputStream(cmdin); } else { terminal = TerminalBuilder.builder().inputStreamWrapper(in -> { @@ -1252,14 +1252,14 @@ private History getHistory() { return in.getHistory(); } - private static final class TestTerminal extends LineDisciplineTerminal { + private static final class NonSystemInTerminal extends LineDisciplineTerminal { private static final int DEFAULT_HEIGHT = 24; private final NonBlockingReader inputReader; - public TestTerminal(InputStream input, OutputStream output) throws Exception { - super("test", "ansi", output, Charset.forName("UTF-8")); + public NonSystemInTerminal(InputStream input, OutputStream output) throws Exception { + super("non-system-in", "ansi", output, Charset.forName("UTF-8")); this.inputReader = NonBlocking.nonBlocking(getName(), input, encoding()); Attributes a = new Attributes(getAttributes()); a.setLocalFlag(LocalFlag.ECHO, false); @@ -1267,7 +1267,7 @@ public TestTerminal(InputStream input, OutputStream output) throws Exception { int h = DEFAULT_HEIGHT; try { String hp = System.getProperty("test.terminal.height"); - if (hp != null && !hp.isEmpty()) { + if (hp != null && !hp.isEmpty() && System.getProperty("test.jdk") != null) { h = Integer.parseInt(hp); } } catch (Throwable ex) { diff --git a/test/langtools/jdk/jshell/CustomInputToolBuilder.java b/test/langtools/jdk/jshell/CustomInputToolBuilder.java new file mode 100644 index 0000000000000..22d9a5bc3ad8a --- /dev/null +++ b/test/langtools/jdk/jshell/CustomInputToolBuilder.java @@ -0,0 +1,74 @@ +/* + * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8247403 + * @summary Verify JavaShellToolBuilder uses provided inputs + * @modules jdk.jshell + * @build KullaTesting TestingInputStream + * @run testng CustomInputToolBuilder + */ + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.io.PrintStream; +import jdk.jshell.tool.JavaShellToolBuilder; +import org.testng.annotations.Test; + +import static org.testng.Assert.assertTrue; + +@Test +public class CustomInputToolBuilder extends KullaTesting { + + private static final String TEST_JDK = "test.jdk"; + + public void checkCustomInput() throws Exception { + String testJdk = System.getProperty(TEST_JDK); + try { + System.clearProperty(TEST_JDK); + byte[] cmdInputData = "System.out.println(\"read: \" + System.in.read());\n/exit\n".getBytes(); + InputStream cmdInput = new ByteArrayInputStream(cmdInputData); + InputStream userInput = new ByteArrayInputStream("a\n".getBytes()); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + PrintStream printOut = new PrintStream(out); + + JavaShellToolBuilder.builder() + .in(cmdInput, userInput) + .out(printOut, new PrintStream(new ByteArrayOutputStream()), printOut) + .promptCapture(true) + .start("--no-startup"); + + String expected = "read: 97"; + String actual = new String(out.toByteArray()); + + assertTrue(actual.contains(expected), + "actual:\n" + actual + "\n, expected:\n" + expected); + } finally { + System.setProperty(TEST_JDK, testJdk); + } + } + +} + From d2ea269f6f731259264af09e88b56cdf58d83ae2 Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Tue, 13 Apr 2021 13:13:16 +0200 Subject: [PATCH 2/7] When using JavaShellToolBuilder programmatically, make the characters sent to the output stream more consistent with the legacy behavior. --- .../org/jline/reader/impl/LineReaderImpl.java | 2 +- .../jshell/tool/ConsoleIOContext.java | 71 ++++++++++++++----- .../jdk/internal/jshell/tool/JShellTool.java | 6 +- .../jdk/jshell/CustomInputToolBuilder.java | 40 ++++++++--- test/langtools/jdk/jshell/HistoryUITest.java | 1 + test/langtools/jdk/jshell/IndentUITest.java | 1 + .../jshell/PasteAndMeasurementsUITest.java | 1 + .../ToolMultilineSnippetHistoryTest.java | 1 + .../jdk/jshell/ToolShiftTabTest.java | 1 + .../jdk/jshell/ToolTabCommandTest.java | 1 + .../jdk/jshell/ToolTabSnippetTest.java | 2 +- test/langtools/jdk/jshell/UITesting.java | 10 ++- 12 files changed, 106 insertions(+), 31 deletions(-) diff --git a/src/jdk.internal.le/share/classes/jdk/internal/org/jline/reader/impl/LineReaderImpl.java b/src/jdk.internal.le/share/classes/jdk/internal/org/jline/reader/impl/LineReaderImpl.java index e5e8efc2471fd..b6592b688193f 100644 --- a/src/jdk.internal.le/share/classes/jdk/internal/org/jline/reader/impl/LineReaderImpl.java +++ b/src/jdk.internal.le/share/classes/jdk/internal/org/jline/reader/impl/LineReaderImpl.java @@ -4157,7 +4157,7 @@ private AttributedString expandPromptPattern(String pattern, int padToWidth, } else sb.append(ch); } - if (padToWidth > cols) { + if (padToWidth > cols && padToWidth > 0) { int padCharCols = WCWidth.wcwidth(padChar); int padCount = (padToWidth - cols) / padCharCols; sb = padPartString; diff --git a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java index ba8a776a7977f..ae4e7926142cc 100644 --- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java +++ b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java @@ -47,6 +47,7 @@ import java.util.ListIterator; import java.util.Map; import java.util.Optional; +import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -104,7 +105,6 @@ class ConsoleIOContext extends IOContext { String prefix = ""; ConsoleIOContext(JShellTool repl, InputStream cmdin, PrintStream cmdout) throws Exception { - this.allowIncompleteInputs = Boolean.getBoolean("jshell.test.allow.incomplete.inputs"); this.repl = repl; Map variables = new HashMap<>(); this.input = new StopDetectingInputStream(() -> repl.stop(), @@ -116,8 +116,16 @@ public int readBuffered(byte[] b) throws IOException { } }; Terminal terminal; + boolean allowIncompleteInputs = Boolean.getBoolean("jshell.test.allow.incomplete.inputs"); + Consumer setupReader = r -> {}; if (cmdin != System.in) { - terminal = new NonSystemInTerminal(nonBlockingInput, cmdout); + if (System.getProperty("test.jdk") != null) { + terminal = new TestTerminal(nonBlockingInput, cmdout); + } else { + terminal = new ProgrammaticInTerminal(nonBlockingInput, cmdout); + setupReader = r -> r.unsetOpt(Option.BRACKETED_PASTE); + allowIncompleteInputs = true; + } input.setInputStream(cmdin); } else { terminal = TerminalBuilder.builder().inputStreamWrapper(in -> { @@ -125,6 +133,7 @@ public int readBuffered(byte[] b) throws IOException { return nonBlockingInput; }).build(); } + this.allowIncompleteInputs = allowIncompleteInputs; originalAttributes = terminal.getAttributes(); Attributes noIntr = new Attributes(originalAttributes); noIntr.setControlChar(ControlChar.VINTR, 0); @@ -179,10 +188,11 @@ protected boolean insertCloseSquare() { } }; + setupReader.accept(reader); reader.setOpt(Option.DISABLE_EVENT_EXPANSION); reader.setParser((line, cursor, context) -> { - if (!allowIncompleteInputs && !repl.isComplete(line)) { + if (!ConsoleIOContext.this.allowIncompleteInputs && !repl.isComplete(line)) { int pendingBraces = countPendingOpenBraces(line); throw new EOFError(cursor, cursor, line, null, pendingBraces, null); } @@ -230,7 +240,7 @@ public String readLine(String firstLinePrompt, String continuationPrompt, this.prefix = prefix; try { in.setVariable(LineReader.SECONDARY_PROMPT_PATTERN, continuationPrompt); - return in.readLine(firstLinePrompt); + return in.readLine(firstLine ? firstLinePrompt : continuationPrompt); } catch (UserInterruptException ex) { throw (InputInterruptedException) new InputInterruptedException().initCause(ex); } catch (EndOfFileException ex) { @@ -1252,28 +1262,28 @@ private History getHistory() { return in.getHistory(); } - private static final class NonSystemInTerminal extends LineDisciplineTerminal { + private static class ProgrammaticInTerminal extends LineDisciplineTerminal { - private static final int DEFAULT_HEIGHT = 24; + protected static final int DEFAULT_HEIGHT = 24; private final NonBlockingReader inputReader; + private final Size bufferSize; + + public ProgrammaticInTerminal(InputStream input, OutputStream output) throws Exception { + this(input, output, "dumb", + new Size(80, DEFAULT_HEIGHT), + new Size(Integer.MAX_VALUE - 1, DEFAULT_HEIGHT)); + } - public NonSystemInTerminal(InputStream input, OutputStream output) throws Exception { - super("non-system-in", "ansi", output, Charset.forName("UTF-8")); + protected ProgrammaticInTerminal(InputStream input, OutputStream output, + String terminal, Size size, Size bufferSize) throws Exception { + super("non-system-in", terminal, output, Charset.forName("UTF-8")); this.inputReader = NonBlocking.nonBlocking(getName(), input, encoding()); Attributes a = new Attributes(getAttributes()); a.setLocalFlag(LocalFlag.ECHO, false); setAttributes(attributes); - int h = DEFAULT_HEIGHT; - try { - String hp = System.getProperty("test.terminal.height"); - if (hp != null && !hp.isEmpty() && System.getProperty("test.jdk") != null) { - h = Integer.parseInt(hp); - } - } catch (Throwable ex) { - // ignore - } - setSize(new Size(80, h)); + setSize(size); + this.bufferSize = bufferSize; } @Override @@ -1288,6 +1298,31 @@ protected void doClose() throws IOException { inputReader.close(); } + @Override + public Size getBufferSize() { + return bufferSize; + } + } + + private static final class TestTerminal extends ProgrammaticInTerminal { + private static Size computeSize() { + int h = DEFAULT_HEIGHT; + try { + String hp = System.getProperty("test.terminal.height"); + if (hp != null && !hp.isEmpty() && System.getProperty("test.jdk") != null) { + h = Integer.parseInt(hp); + } + } catch (Throwable ex) { + // ignore + } + return new Size(80, h); + } + public TestTerminal(InputStream input, OutputStream output) throws Exception { + this(input, output, computeSize()); + } + private TestTerminal(InputStream input, OutputStream output, Size size) throws Exception { + super(input, output, "ansi", size, size); + } } private static final class CompletionState { diff --git a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java index f85dddccfa022..bfa709959ae31 100644 --- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java +++ b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java @@ -138,6 +138,8 @@ */ public class JShellTool implements MessageHandler { + private static String PROMPT = "\u0005"; + private static String CONTINUATION_PROMPT = "\u0006"; private static final Pattern LINEBREAK = Pattern.compile("\\R"); private static final Pattern ID = Pattern.compile("[se]?\\d+([-\\s].*)?"); private static final Pattern RERUN_ID = Pattern.compile("/" + ID.pattern()); @@ -1256,12 +1258,12 @@ private String getInput(String initial) throws IOException{ return src; } String firstLinePrompt = interactive() - ? testPrompt ? " \005" + ? testPrompt ? PROMPT : feedback.getPrompt(currentNameSpace.tidNext()) : "" // Non-interactive -- no prompt ; String continuationPrompt = interactive() - ? testPrompt ? " \006" + ? testPrompt ? CONTINUATION_PROMPT : feedback.getContinuationPrompt(currentNameSpace.tidNext()) : "" // Non-interactive -- no prompt ; diff --git a/test/langtools/jdk/jshell/CustomInputToolBuilder.java b/test/langtools/jdk/jshell/CustomInputToolBuilder.java index 22d9a5bc3ad8a..ddf4852782b8c 100644 --- a/test/langtools/jdk/jshell/CustomInputToolBuilder.java +++ b/test/langtools/jdk/jshell/CustomInputToolBuilder.java @@ -34,6 +34,8 @@ import java.io.ByteArrayOutputStream; import java.io.InputStream; import java.io.PrintStream; +import java.util.Arrays; +import java.util.List; import jdk.jshell.tool.JavaShellToolBuilder; import org.testng.annotations.Test; @@ -48,7 +50,30 @@ public void checkCustomInput() throws Exception { String testJdk = System.getProperty(TEST_JDK); try { System.clearProperty(TEST_JDK); - byte[] cmdInputData = "System.out.println(\"read: \" + System.in.read());\n/exit\n".getBytes(); + doTest("System.out.println(\"read: \" + System.in.read());", + "\u0005System.out.println(\"read: \" + System.in.read());", + "read: 97", + "\u0005/exit"); + doTest("1 + 1", "\u00051 + 1", "$1 ==> 2", "\u0005/exit"); + doTest("for (int i = 0; i < 100; i++) {\nSystem.err.println(i);\n}\n", + "\u0005for (int i = 0; i < 100; i++) {", + "\u0006System.err.println(i);", "\u0006}", + "\u0005/exit"); + StringBuilder longInput = new StringBuilder(); + String constant = "1_______________1"; + longInput.append(constant); + for (int i = 0; i < 100; i++) { + longInput.append(" + "); + longInput.append(constant); + } + doTest(longInput.toString(), "\u0005" + longInput); + } finally { + System.setProperty(TEST_JDK, testJdk); + } + } + + private void doTest(String code, String... expectedLines) throws Exception { + byte[] cmdInputData = (code + "\n/exit\n").getBytes(); InputStream cmdInput = new ByteArrayInputStream(cmdInputData); InputStream userInput = new ByteArrayInputStream("a\n".getBytes()); ByteArrayOutputStream out = new ByteArrayOutputStream(); @@ -56,19 +81,18 @@ public void checkCustomInput() throws Exception { JavaShellToolBuilder.builder() .in(cmdInput, userInput) - .out(printOut, new PrintStream(new ByteArrayOutputStream()), printOut) + .out(printOut, printOut, printOut) .promptCapture(true) .start("--no-startup"); String expected = "read: 97"; String actual = new String(out.toByteArray()); + List actualLines = Arrays.asList(actual.split("\\R")); - assertTrue(actual.contains(expected), - "actual:\n" + actual + "\n, expected:\n" + expected); - } finally { - System.setProperty(TEST_JDK, testJdk); - } + for (String expectedLine : expectedLines) { + assertTrue(actualLines.contains(expectedLine), + "actual:\n" + actualLines + "\n, expected:\n" + expectedLine); + } } - } diff --git a/test/langtools/jdk/jshell/HistoryUITest.java b/test/langtools/jdk/jshell/HistoryUITest.java index 04b8e825862c7..42e242fe1e6e8 100644 --- a/test/langtools/jdk/jshell/HistoryUITest.java +++ b/test/langtools/jdk/jshell/HistoryUITest.java @@ -28,6 +28,7 @@ * @modules * jdk.compiler/com.sun.tools.javac.api * jdk.compiler/com.sun.tools.javac.main + * jdk.jshell/jdk.internal.jshell.tool:open * jdk.jshell/jdk.internal.jshell.tool.resources:open * jdk.jshell/jdk.jshell:open * @library /tools/lib diff --git a/test/langtools/jdk/jshell/IndentUITest.java b/test/langtools/jdk/jshell/IndentUITest.java index 4f39cbbfc42e0..6858277a179fd 100644 --- a/test/langtools/jdk/jshell/IndentUITest.java +++ b/test/langtools/jdk/jshell/IndentUITest.java @@ -29,6 +29,7 @@ * @modules * jdk.compiler/com.sun.tools.javac.api * jdk.compiler/com.sun.tools.javac.main + * jdk.jshell/jdk.internal.jshell.tool:open * jdk.jshell/jdk.internal.jshell.tool.resources:open * jdk.jshell/jdk.jshell:open * @build toolbox.ToolBox toolbox.JarTask toolbox.JavacTask diff --git a/test/langtools/jdk/jshell/PasteAndMeasurementsUITest.java b/test/langtools/jdk/jshell/PasteAndMeasurementsUITest.java index d6fb13254f4be..932be4f1f7d52 100644 --- a/test/langtools/jdk/jshell/PasteAndMeasurementsUITest.java +++ b/test/langtools/jdk/jshell/PasteAndMeasurementsUITest.java @@ -32,6 +32,7 @@ * jdk.compiler/com.sun.tools.javac.api * jdk.compiler/com.sun.tools.javac.main * jdk.internal.le/jdk.internal.org.jline.reader.impl + * jdk.jshell/jdk.internal.jshell.tool:open * jdk.jshell/jdk.internal.jshell.tool.resources:open * jdk.jshell/jdk.jshell:open * @build toolbox.ToolBox toolbox.JarTask toolbox.JavacTask diff --git a/test/langtools/jdk/jshell/ToolMultilineSnippetHistoryTest.java b/test/langtools/jdk/jshell/ToolMultilineSnippetHistoryTest.java index 85320f59626a5..8a7b46fc1c77c 100644 --- a/test/langtools/jdk/jshell/ToolMultilineSnippetHistoryTest.java +++ b/test/langtools/jdk/jshell/ToolMultilineSnippetHistoryTest.java @@ -26,6 +26,7 @@ * @bug 8182489 * @summary test history with multiline snippets * @modules + * jdk.jshell/jdk.internal.jshell.tool:open * jdk.jshell/jdk.internal.jshell.tool.resources:open * jdk.jshell/jdk.jshell:open * @build UITesting diff --git a/test/langtools/jdk/jshell/ToolShiftTabTest.java b/test/langtools/jdk/jshell/ToolShiftTabTest.java index b668ff500c104..7d8b4be3b7a42 100644 --- a/test/langtools/jdk/jshell/ToolShiftTabTest.java +++ b/test/langtools/jdk/jshell/ToolShiftTabTest.java @@ -26,6 +26,7 @@ * @bug 8166334 8188894 * @summary test shift-tab shortcuts "fixes" * @modules + * jdk.jshell/jdk.internal.jshell.tool:open * jdk.jshell/jdk.internal.jshell.tool.resources:open * jdk.jshell/jdk.jshell:open * @build UITesting diff --git a/test/langtools/jdk/jshell/ToolTabCommandTest.java b/test/langtools/jdk/jshell/ToolTabCommandTest.java index d4fa6dc91022f..9a429bb725300 100644 --- a/test/langtools/jdk/jshell/ToolTabCommandTest.java +++ b/test/langtools/jdk/jshell/ToolTabCommandTest.java @@ -27,6 +27,7 @@ * @modules * jdk.compiler/com.sun.tools.javac.api * jdk.compiler/com.sun.tools.javac.main + * jdk.jshell/jdk.internal.jshell.tool:open * jdk.jshell/jdk.internal.jshell.tool.resources:open * jdk.jshell/jdk.jshell:open * @library /tools/lib diff --git a/test/langtools/jdk/jshell/ToolTabSnippetTest.java b/test/langtools/jdk/jshell/ToolTabSnippetTest.java index f98de0f5e5c7f..ec869da65964a 100644 --- a/test/langtools/jdk/jshell/ToolTabSnippetTest.java +++ b/test/langtools/jdk/jshell/ToolTabSnippetTest.java @@ -27,7 +27,7 @@ * @modules * jdk.compiler/com.sun.tools.javac.api * jdk.compiler/com.sun.tools.javac.main - * jdk.jshell/jdk.internal.jshell.tool + * jdk.jshell/jdk.internal.jshell.tool:+open * jdk.jshell/jdk.internal.jshell.tool.resources:open * jdk.jshell/jdk.jshell:open * @library /tools/lib diff --git a/test/langtools/jdk/jshell/UITesting.java b/test/langtools/jdk/jshell/UITesting.java index f57bbe33bbb5f..1014030e4eb32 100644 --- a/test/langtools/jdk/jshell/UITesting.java +++ b/test/langtools/jdk/jshell/UITesting.java @@ -27,6 +27,7 @@ import java.io.OutputStreamWriter; import java.io.PrintStream; import java.io.Writer; +import java.lang.reflect.Field; import java.text.MessageFormat; import java.util.HashMap; import java.util.Locale; @@ -108,6 +109,13 @@ public void write(String str) throws IOException { runner.start(); try { + Class jshellToolClass = Class.forName("jdk.internal.jshell.tool.JShellTool"); + Field promptField = jshellToolClass.getDeclaredField("PROMPT"); + promptField.setAccessible(true); + promptField.set(null, PROMPT); + Field continuationPromptField = jshellToolClass.getDeclaredField("CONTINUATION_PROMPT"); + continuationPromptField.setAccessible(true); + continuationPromptField.set(null, CONTINUATION_PROMPT); waitOutput(out, PROMPT); test.test(inputSink, out); } finally { @@ -134,7 +142,7 @@ protected interface Test { } catch (NumberFormatException ex) { factor = 1; } - TIMEOUT = 60_000 * factor; + TIMEOUT = 1_000 * factor; } protected void waitOutput(StringBuilder out, String expected) { From 32153743d6f762ec9f1019a49f15d74f307873a0 Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Fri, 16 Apr 2021 13:09:59 +0200 Subject: [PATCH 3/7] Adding an experimental API to select the JavaShell output format and the output terminal size. --- .../jshell/tool/ConsoleIOContext.java | 27 ++++++++---- .../jdk/internal/jshell/tool/JShellTool.java | 11 ++++- .../jshell/tool/JShellToolBuilder.java | 18 +++++++- .../jdk/jshell/tool/JavaShellToolBuilder.java | 43 +++++++++++++++++++ 4 files changed, 88 insertions(+), 11 deletions(-) diff --git a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java index ae4e7926142cc..aaf9a115a05ca 100644 --- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java +++ b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java @@ -104,7 +104,8 @@ class ConsoleIOContext extends IOContext { String prefix = ""; - ConsoleIOContext(JShellTool repl, InputStream cmdin, PrintStream cmdout) throws Exception { + ConsoleIOContext(JShellTool repl, InputStream cmdin, PrintStream cmdout, + boolean interactive, int columns, int rows) throws Exception { this.repl = repl; Map variables = new HashMap<>(); this.input = new StopDetectingInputStream(() -> repl.stop(), @@ -122,9 +123,16 @@ public int readBuffered(byte[] b) throws IOException { if (System.getProperty("test.jdk") != null) { terminal = new TestTerminal(nonBlockingInput, cmdout); } else { - terminal = new ProgrammaticInTerminal(nonBlockingInput, cmdout); - setupReader = r -> r.unsetOpt(Option.BRACKETED_PASTE); - allowIncompleteInputs = true; + Size size = null; + if (columns != (-1) && rows != (-1)) { + size = new Size(columns, rows); + } + terminal = new ProgrammaticInTerminal(nonBlockingInput, cmdout, interactive, + size); + if (!interactive) { + setupReader = r -> r.unsetOpt(Option.BRACKETED_PASTE); + allowIncompleteInputs = true; + } } input.setInputStream(cmdin); } else { @@ -1269,10 +1277,13 @@ private static class ProgrammaticInTerminal extends LineDisciplineTerminal { private final NonBlockingReader inputReader; private final Size bufferSize; - public ProgrammaticInTerminal(InputStream input, OutputStream output) throws Exception { - this(input, output, "dumb", - new Size(80, DEFAULT_HEIGHT), - new Size(Integer.MAX_VALUE - 1, DEFAULT_HEIGHT)); + public ProgrammaticInTerminal(InputStream input, OutputStream output, + boolean interactive, Size size) throws Exception { + this(input, output, interactive ? "ansi" : "dumb", + size != null ? size : new Size(80, DEFAULT_HEIGHT), + size != null ? size + : interactive ? new Size(80, DEFAULT_HEIGHT) + : new Size(Integer.MAX_VALUE - 1, DEFAULT_HEIGHT)); } protected ProgrammaticInTerminal(InputStream input, OutputStream output, diff --git a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java index bfa709959ae31..9095d0ead471a 100644 --- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java +++ b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java @@ -160,6 +160,9 @@ public class JShellTool implements MessageHandler { final PersistentStorage prefs; final Map envvars; final Locale locale; + final boolean interactiveTerminal; + final int terminalColumns; + final int terminalRows; final Feedback feedback = new Feedback(); @@ -179,7 +182,8 @@ public class JShellTool implements MessageHandler { JShellTool(InputStream cmdin, PrintStream cmdout, PrintStream cmderr, PrintStream console, InputStream userin, PrintStream userout, PrintStream usererr, - PersistentStorage prefs, Map envvars, Locale locale) { + PersistentStorage prefs, Map envvars, Locale locale, + boolean interactiveTerminal, int terminalColumns, int terminalRows) { this.cmdin = cmdin; this.cmdout = cmdout; this.cmderr = cmderr; @@ -195,6 +199,9 @@ public int read() throws IOException { this.prefs = prefs; this.envvars = envvars; this.locale = locale; + this.interactiveTerminal = interactiveTerminal; + this.terminalColumns = terminalColumns; + this.terminalRows = terminalRows; } private ResourceBundle versionRB = null; @@ -976,7 +983,7 @@ public void run() { }; Runtime.getRuntime().addShutdownHook(shutdownHook); // execute from user input - try (IOContext in = new ConsoleIOContext(this, cmdin, console)) { + try (IOContext in = new ConsoleIOContext(this, cmdin, console, interactiveTerminal, terminalColumns, terminalRows)) { int indent; try { String indentValue = indent(); diff --git a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellToolBuilder.java b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellToolBuilder.java index b469b20fa88c8..eac3c9be00f7a 100644 --- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellToolBuilder.java +++ b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellToolBuilder.java @@ -51,6 +51,9 @@ public class JShellToolBuilder implements JavaShellToolBuilder { private PersistentStorage prefs = null; private Map vars = null; private Locale locale = Locale.getDefault(); + private boolean interactiveTerminal; + private int terminalColumns = -1; + private int terminalRows = -1; private boolean capturePrompt = false; /** @@ -208,6 +211,19 @@ public JavaShellToolBuilder promptCapture(boolean capture) { return this; } + @Override + public JavaShellToolBuilder interactiveTerminal(boolean terminal) { + this.interactiveTerminal = terminal; + return this; + } + + @Override + public JavaShellToolBuilder terminalSize(int columns, int rows) { + this.terminalColumns = columns; + this.terminalRows = rows; + return this; + } + /** * Create a tool instance for testing. Not in JavaShellToolBuilder. * @@ -221,7 +237,7 @@ public JShellTool rawTool() { vars = System.getenv(); } JShellTool sh = new JShellTool(cmdIn, cmdOut, cmdErr, console, userIn, - userOut, userErr, prefs, vars, locale); + userOut, userErr, prefs, vars, locale, interactiveTerminal, terminalColumns, terminalRows); sh.testPrompt = capturePrompt; return sh; } diff --git a/src/jdk.jshell/share/classes/jdk/jshell/tool/JavaShellToolBuilder.java b/src/jdk.jshell/share/classes/jdk/jshell/tool/JavaShellToolBuilder.java index 5297136bf9ac6..1b2bd93a3cf68 100644 --- a/src/jdk.jshell/share/classes/jdk/jshell/tool/JavaShellToolBuilder.java +++ b/src/jdk.jshell/share/classes/jdk/jshell/tool/JavaShellToolBuilder.java @@ -183,6 +183,49 @@ static JavaShellToolBuilder builder() { */ JavaShellToolBuilder promptCapture(boolean capture); + /** + * Set to true to specify the inputs and outputs are connected to an interactive terminal + * that can interpret the ANSI escape codes. The characters sent to the output streams are + * assumed to be interpreted by a terminal and shown to the user, and the exact order and nature + * of characters sent to the outputs are unspecified. + * + * Set to false to specify a legacy simpler behavior whose output can be parsed by automatic + * tools. + * + * When the input stream for this Java Shell is {@code System.in}, this value is ignored, + * and the behavior is similar to specifying {@code true} in this method, but is more closely + * following the specific terminal connected to {@code System.in}. + * + * @implSpec If this method is not called, the behavior should be + * equivalent to calling {@code interactiveTerminal(false)}. + * + * @param terminal if {@code true}, an terminal that can interpret the ANSI escape codes is + * assumed to interpret the output. If {@code false}, a simpler output is selected. + * @return the {@code JavaShellToolBuilder} instance + * @since 17 + */ + default JavaShellToolBuilder interactiveTerminal(boolean terminal) { + return this; + } + + /** + * Set to number of columns and rows of the terminal to which the output of this Java Shell + * is attached. + * + * When the input stream for this Java Shell is {@code System.in}, these values are ignored, + * and the size of the terminal is auto-detected. + * + * @implSpec If this method is not called, default values are chosen. + * + * @param columns the number of columns of the attached terminal + * @param rows the number of rows of the attached terminal + * @return the {@code JavaShellToolBuilder} instance + * @since 17 + */ + default JavaShellToolBuilder terminalSize(int columns, int rows) { + return this; + } + /** * Run an instance of the Java shell tool as configured by the other methods * in this interface. This call is not destructive, more than one call of From 4bf4af309e831a6980dd027e6ee04ba4db760907 Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Mon, 26 Apr 2021 10:03:16 +0200 Subject: [PATCH 4/7] Cleanup. --- .../internal/jshell/tool/ConsoleIOContext.java | 5 +---- .../jdk/internal/jshell/tool/JShellTool.java | 8 ++------ .../jshell/tool/JShellToolBuilder.java | 11 +---------- .../jdk/jshell/tool/JavaShellToolBuilder.java | 18 ------------------ 4 files changed, 4 insertions(+), 38 deletions(-) diff --git a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java index aaf9a115a05ca..71fae166781df 100644 --- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java +++ b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java @@ -105,7 +105,7 @@ class ConsoleIOContext extends IOContext { String prefix = ""; ConsoleIOContext(JShellTool repl, InputStream cmdin, PrintStream cmdout, - boolean interactive, int columns, int rows) throws Exception { + boolean interactive) throws Exception { this.repl = repl; Map variables = new HashMap<>(); this.input = new StopDetectingInputStream(() -> repl.stop(), @@ -124,9 +124,6 @@ public int readBuffered(byte[] b) throws IOException { terminal = new TestTerminal(nonBlockingInput, cmdout); } else { Size size = null; - if (columns != (-1) && rows != (-1)) { - size = new Size(columns, rows); - } terminal = new ProgrammaticInTerminal(nonBlockingInput, cmdout, interactive, size); if (!interactive) { diff --git a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java index 9095d0ead471a..532454b7cbcc1 100644 --- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java +++ b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java @@ -161,8 +161,6 @@ public class JShellTool implements MessageHandler { final Map envvars; final Locale locale; final boolean interactiveTerminal; - final int terminalColumns; - final int terminalRows; final Feedback feedback = new Feedback(); @@ -183,7 +181,7 @@ public class JShellTool implements MessageHandler { PrintStream console, InputStream userin, PrintStream userout, PrintStream usererr, PersistentStorage prefs, Map envvars, Locale locale, - boolean interactiveTerminal, int terminalColumns, int terminalRows) { + boolean interactiveTerminal) { this.cmdin = cmdin; this.cmdout = cmdout; this.cmderr = cmderr; @@ -200,8 +198,6 @@ public int read() throws IOException { this.envvars = envvars; this.locale = locale; this.interactiveTerminal = interactiveTerminal; - this.terminalColumns = terminalColumns; - this.terminalRows = terminalRows; } private ResourceBundle versionRB = null; @@ -983,7 +979,7 @@ public void run() { }; Runtime.getRuntime().addShutdownHook(shutdownHook); // execute from user input - try (IOContext in = new ConsoleIOContext(this, cmdin, console, interactiveTerminal, terminalColumns, terminalRows)) { + try (IOContext in = new ConsoleIOContext(this, cmdin, console, interactiveTerminal)) { int indent; try { String indentValue = indent(); diff --git a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellToolBuilder.java b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellToolBuilder.java index eac3c9be00f7a..52e042d365a02 100644 --- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellToolBuilder.java +++ b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellToolBuilder.java @@ -52,8 +52,6 @@ public class JShellToolBuilder implements JavaShellToolBuilder { private Map vars = null; private Locale locale = Locale.getDefault(); private boolean interactiveTerminal; - private int terminalColumns = -1; - private int terminalRows = -1; private boolean capturePrompt = false; /** @@ -217,13 +215,6 @@ public JavaShellToolBuilder interactiveTerminal(boolean terminal) { return this; } - @Override - public JavaShellToolBuilder terminalSize(int columns, int rows) { - this.terminalColumns = columns; - this.terminalRows = rows; - return this; - } - /** * Create a tool instance for testing. Not in JavaShellToolBuilder. * @@ -237,7 +228,7 @@ public JShellTool rawTool() { vars = System.getenv(); } JShellTool sh = new JShellTool(cmdIn, cmdOut, cmdErr, console, userIn, - userOut, userErr, prefs, vars, locale, interactiveTerminal, terminalColumns, terminalRows); + userOut, userErr, prefs, vars, locale, interactiveTerminal); sh.testPrompt = capturePrompt; return sh; } diff --git a/src/jdk.jshell/share/classes/jdk/jshell/tool/JavaShellToolBuilder.java b/src/jdk.jshell/share/classes/jdk/jshell/tool/JavaShellToolBuilder.java index 1b2bd93a3cf68..1491590467b0a 100644 --- a/src/jdk.jshell/share/classes/jdk/jshell/tool/JavaShellToolBuilder.java +++ b/src/jdk.jshell/share/classes/jdk/jshell/tool/JavaShellToolBuilder.java @@ -208,24 +208,6 @@ default JavaShellToolBuilder interactiveTerminal(boolean terminal) { return this; } - /** - * Set to number of columns and rows of the terminal to which the output of this Java Shell - * is attached. - * - * When the input stream for this Java Shell is {@code System.in}, these values are ignored, - * and the size of the terminal is auto-detected. - * - * @implSpec If this method is not called, default values are chosen. - * - * @param columns the number of columns of the attached terminal - * @param rows the number of rows of the attached terminal - * @return the {@code JavaShellToolBuilder} instance - * @since 17 - */ - default JavaShellToolBuilder terminalSize(int columns, int rows) { - return this; - } - /** * Run an instance of the Java shell tool as configured by the other methods * in this interface. This call is not destructive, more than one call of From 160cd3c0b0f31e737085d930861632a286ecd96f Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Mon, 26 Apr 2021 13:11:24 +0200 Subject: [PATCH 5/7] Improving test. --- .../jdk/jshell/CustomInputToolBuilder.java | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/test/langtools/jdk/jshell/CustomInputToolBuilder.java b/test/langtools/jdk/jshell/CustomInputToolBuilder.java index ddf4852782b8c..a80f58daabe24 100644 --- a/test/langtools/jdk/jshell/CustomInputToolBuilder.java +++ b/test/langtools/jdk/jshell/CustomInputToolBuilder.java @@ -73,6 +73,10 @@ public void checkCustomInput() throws Exception { } private void doTest(String code, String... expectedLines) throws Exception { + doTest(false, code, expectedLines); + } + + private void doTest(boolean interactiveTerminal, String code, String... expectedLines) throws Exception { byte[] cmdInputData = (code + "\n/exit\n").getBytes(); InputStream cmdInput = new ByteArrayInputStream(cmdInputData); InputStream userInput = new ByteArrayInputStream("a\n".getBytes()); @@ -82,10 +86,10 @@ private void doTest(String code, String... expectedLines) throws Exception { JavaShellToolBuilder.builder() .in(cmdInput, userInput) .out(printOut, printOut, printOut) + .interactiveTerminal(interactiveTerminal) .promptCapture(true) .start("--no-startup"); - String expected = "read: 97"; String actual = new String(out.toByteArray()); List actualLines = Arrays.asList(actual.split("\\R")); @@ -94,5 +98,26 @@ private void doTest(String code, String... expectedLines) throws Exception { "actual:\n" + actualLines + "\n, expected:\n" + expectedLine); } } -} + public void checkInteractiveTerminal() throws Exception { + String testJdk = System.getProperty(TEST_JDK); + try { + System.clearProperty(TEST_JDK); + + //note the exact format of the output is not specified here, and the test mostly validates + //the current behavior, and shows the output changes based on the interactiveTerminal setting: + doTest(true, + "System.out.println(\"read: \" + System.in.read());", + "\u001b[?2004h\u0005System.out.println(\"read: \" + System.in.read()\u001b[2D\u001b[2C)\u001b[29D\u001b[29C;", + "\u001b[?2004lread: 97", + "\u001b[?2004h\u0005/exit"); + doTest(true, + "1 + 1", + "\u001b[?2004h\u00051 + 1", + "\u001b[?2004l$1 ==> 2", + "\u001b[?2004h\u0005/exit"); + } finally { + System.setProperty(TEST_JDK, testJdk); + } + } +} From d7eacd6a8bbd535a352f73fffc579041af0b630f Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Mon, 26 Apr 2021 13:22:59 +0200 Subject: [PATCH 6/7] Restoring timeout. --- test/langtools/jdk/jshell/UITesting.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/langtools/jdk/jshell/UITesting.java b/test/langtools/jdk/jshell/UITesting.java index 1014030e4eb32..71200cc1a476d 100644 --- a/test/langtools/jdk/jshell/UITesting.java +++ b/test/langtools/jdk/jshell/UITesting.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -142,7 +142,7 @@ protected interface Test { } catch (NumberFormatException ex) { factor = 1; } - TIMEOUT = 1_000 * factor; + TIMEOUT = 60_000 * factor; } protected void waitOutput(StringBuilder out, String expected) { From 6ee9855c1fffb1a9e7e09022f202bd27ddb92113 Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Mon, 24 May 2021 10:14:52 +0200 Subject: [PATCH 7/7] Adding note about the default behavior, as suggested. --- .../share/classes/jdk/jshell/tool/JavaShellToolBuilder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/jdk.jshell/share/classes/jdk/jshell/tool/JavaShellToolBuilder.java b/src/jdk.jshell/share/classes/jdk/jshell/tool/JavaShellToolBuilder.java index 1491590467b0a..c4b4bf66b336a 100644 --- a/src/jdk.jshell/share/classes/jdk/jshell/tool/JavaShellToolBuilder.java +++ b/src/jdk.jshell/share/classes/jdk/jshell/tool/JavaShellToolBuilder.java @@ -197,7 +197,8 @@ static JavaShellToolBuilder builder() { * following the specific terminal connected to {@code System.in}. * * @implSpec If this method is not called, the behavior should be - * equivalent to calling {@code interactiveTerminal(false)}. + * equivalent to calling {@code interactiveTerminal(false)}. The default implementation of + * this method returns {@code this}. * * @param terminal if {@code true}, an terminal that can interpret the ANSI escape codes is * assumed to interpret the output. If {@code false}, a simpler output is selected.