8172179: jshell tool: builtin startup settings should be by reference not content
Reviewed-by: jlahoda
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java Thu Jan 19 07:02:34 2017 -0800
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java Thu Jan 19 11:12:02 2017 -0800
@@ -128,9 +128,8 @@
*/
public class JShellTool implements MessageHandler {
- private static final String LINE_SEP = System.getProperty("line.separator");
private static final Pattern LINEBREAK = Pattern.compile("\\R");
- private static final String RECORD_SEPARATOR = "\u241E";
+ static final String RECORD_SEPARATOR = "\u241E";
private static final String RB_NAME_PREFIX = "jdk.internal.jshell.tool.resources";
private static final String VERSION_RB_NAME = RB_NAME_PREFIX + ".version";
private static final String L10N_RB_NAME = RB_NAME_PREFIX + ".l10n";
@@ -199,7 +198,7 @@
private boolean debug = false;
public boolean testPrompt = false;
private String defaultStartup = null;
- private String startup = null;
+ private Startup startup = null;
private String executionControlSpec = null;
private EditorSetting editor = BUILT_IN_EDITOR;
@@ -216,7 +215,6 @@
static final String MODE_KEY = "MODE";
static final String REPLAY_RESTORE_KEY = "REPLAY_RESTORE";
- static final String DEFAULT_STARTUP_NAME = "DEFAULT";
static final Pattern BUILTIN_FILE_PATTERN = Pattern.compile("\\w+");
static final String BUILTIN_FILE_PATH_FORMAT = "/jdk/jshell/tool/resources/%s.jsh";
@@ -431,13 +429,13 @@
private final OptionSpecBuilder argX = parser.accepts("X");
private String feedbackMode = null;
- private String initialStartup = null;
+ private Startup initialStartup = null;
String feedbackMode() {
return feedbackMode;
}
- String startup() {
+ Startup startup() {
return initialStartup;
}
@@ -485,22 +483,15 @@
startmsg("jshell.err.opt.startup.conflict");
return null;
}
- StringBuilder sb = new StringBuilder();
- for (String fn : sts) {
- String s = readFile(fn, "--startup");
- if (s == null) {
- return null;
- }
- sb.append(s);
+ initialStartup = Startup.fromFileList(sts, "--startup", new InitMessageHandler());
+ if (initialStartup == null) {
+ return null;
}
- initialStartup = sb.toString();
} else if (options.has(argNoStart)) {
- initialStartup = "";
+ initialStartup = Startup.noStartup();
} else {
- initialStartup = prefs.get(STARTUP_KEY);
- if (initialStartup == null) {
- initialStartup = defaultStartup();
- }
+ String packedStartup = prefs.get(STARTUP_KEY);
+ initialStartup = Startup.unpack(packedStartup, new InitMessageHandler());
}
if (options.has(argExecution)) {
executionControlSpec = options.valueOf(argExecution);
@@ -639,6 +630,11 @@
return "";
}
String pp = s.replaceAll("\\R", post + pre);
+ if (pp.endsWith(post + pre)) {
+ // prevent an extra prefix char and blank line when the string
+ // already terminates with newline
+ pp = pp.substring(0, pp.length() - (post + pre).length());
+ }
return pre + pp + post;
}
@@ -893,7 +889,7 @@
analysis = state.sourceCodeAnalysis();
live = true;
- startUpRun(startup);
+ startUpRun(startup.toString());
currentNameSpace = mainNamespace;
}
@@ -1077,7 +1073,7 @@
.toArray(Command[]::new);
}
- private static Path toPathResolvingUserHome(String pathString) {
+ static Path toPathResolvingUserHome(String pathString) {
if (pathString.replace(File.separatorChar, '/').startsWith("~/"))
return Paths.get(System.getProperty("user.home"), pathString.substring(2));
else
@@ -1838,48 +1834,43 @@
return true;
}
if (hasFile) {
- StringBuilder sb = new StringBuilder();
- for (String fn : fns) {
- String s = readFile(fn, "/set start");
- if (s == null) {
- return false;
- }
- sb.append(s);
+ startup = Startup.fromFileList(fns, "/set start", this);
+ if (startup == null) {
+ return false;
}
- startup = sb.toString();
} else if (defaultOption) {
- startup = defaultStartup();
+ startup = Startup.defaultStartup(this);
} else if (noneOption) {
- startup = "";
+ startup = Startup.noStartup();
}
if (retainOption) {
// retain startup setting
- prefs.put(STARTUP_KEY, startup);
+ prefs.put(STARTUP_KEY, startup.storedForm());
}
return true;
}
+ // show the "/set start" settings (retained and, if different, current)
+ // as commands (and file contents). All commands first, then contents.
void showSetStart() {
+ StringBuilder sb = new StringBuilder();
String retained = prefs.get(STARTUP_KEY);
if (retained != null) {
- showSetStart(true, retained);
- }
- if (retained == null || !startup.equals(retained)) {
- showSetStart(false, startup);
+ Startup retainedStart = Startup.unpack(retained, this);
+ boolean currentDifferent = !startup.equals(retainedStart);
+ sb.append(retainedStart.show(true));
+ if (currentDifferent) {
+ sb.append(startup.show(false));
+ }
+ sb.append(retainedStart.showDetail());
+ if (currentDifferent) {
+ sb.append(startup.showDetail());
+ }
+ } else {
+ sb.append(startup.show(false));
+ sb.append(startup.showDetail());
}
- }
-
- void showSetStart(boolean isRetained, String start) {
- String cmd = "/set start" + (isRetained ? " -retain " : " ");
- String stset;
- if (start.equals(defaultStartup())) {
- stset = cmd + "-default";
- } else if (start.isEmpty()) {
- stset = cmd + "-none";
- } else {
- stset = "startup.jsh:\n" + start + "\n" + cmd + "startup.jsh";
- }
- hard(stset);
+ hard(sb.toString());
}
boolean cmdDebug(String arg) {
@@ -2380,38 +2371,7 @@
return false;
}
- /**
- * Read an external file. Error messages accessed via keyPrefix
- *
- * @param filename file to access or null
- * @param context printable non-natural language context for errors
- * @return contents of file as string
- */
- String readFile(String filename, String context) {
- if (filename != null) {
- try {
- byte[] encoded = Files.readAllBytes(toPathResolvingUserHome(filename));
- return new String(encoded);
- } catch (AccessDeniedException e) {
- errormsg("jshell.err.file.not.accessible", context, filename, e.getMessage());
- } catch (NoSuchFileException e) {
- String resource = getResource(filename);
- if (resource != null) {
- // Not found as file, but found as resource
- return resource;
- }
- errormsg("jshell.err.file.not.found", context, filename);
- } catch (Exception e) {
- errormsg("jshell.err.file.exception", context, filename, e);
- }
- } else {
- errormsg("jshell.err.file.filename", context);
- }
- return null;
-
- }
-
- String getResource(String name) {
+ static String getResource(String name) {
if (BUILTIN_FILE_PATTERN.matcher(name).matches()) {
try {
return readResource(name);
@@ -2423,33 +2383,16 @@
}
// Read a built-in file from resources
- String readResource(String name) throws IOException {
+ static String readResource(String name) throws IOException {
// Attempt to find the file as a resource
String spec = String.format(BUILTIN_FILE_PATH_FORMAT, name);
try (InputStream in = JShellTool.class.getResourceAsStream(spec);
- BufferedReader reader = new BufferedReader(new InputStreamReader(in))) {
- return reader.lines().collect(Collectors.joining("\n"));
+ BufferedReader reader = new BufferedReader(new InputStreamReader(in))) {
+ return reader.lines().collect(Collectors.joining("\n", "", "\n"));
}
}
- // retrieve the default startup string
- String defaultStartup() {
- if (defaultStartup == null) {
- defaultStartup = ""; // failure case
- try {
- defaultStartup = readResource(DEFAULT_STARTUP_NAME);
- } catch (AccessDeniedException e) {
- errormsg("jshell.err.file.not.accessible", "jshell", DEFAULT_STARTUP_NAME, e.getMessage());
- } catch (NoSuchFileException e) {
- errormsg("jshell.err.file.not.found", "jshell", DEFAULT_STARTUP_NAME);
- } catch (Exception e) {
- errormsg("jshell.err.file.exception", "jshell", DEFAULT_STARTUP_NAME, e);
- }
- }
- return defaultStartup;
- }
-
private boolean cmdReset(String rawargs) {
if (!parseCommandLineLikeFlags(rawargs, new OptionParserBase())) {
return false;
@@ -2551,7 +2494,7 @@
writer.write("\n");
}
} else if (at.hasOption("-start")) {
- writer.append(startup);
+ writer.append(startup.toString());
} else {
String sources = (at.hasOption("-all")
? state.snippets()
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/Startup.java Thu Jan 19 11:12:02 2017 -0800
@@ -0,0 +1,352 @@
+/*
+ * Copyright (c) 2016, 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. Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+
+package jdk.internal.jshell.tool;
+
+import java.nio.file.AccessDeniedException;
+import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.time.format.FormatStyle;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import static java.util.stream.Collectors.joining;
+import static java.util.stream.Collectors.toList;
+import static jdk.internal.jshell.tool.JShellTool.RECORD_SEPARATOR;
+import static jdk.internal.jshell.tool.JShellTool.getResource;
+import static jdk.internal.jshell.tool.JShellTool.readResource;
+import static jdk.internal.jshell.tool.JShellTool.toPathResolvingUserHome;
+
+/**
+ * Processing start-up "script" information. The startup may consist of several
+ * entries, each of which may have been read from a user file or be a built-in
+ * resource. The startup may also be empty ("-none"); Which is stored as the
+ * empty string different from unset (null). Built-in resources come from
+ * resource files. Startup is stored as named elements rather than concatenated
+ * text, for display purposes but also importantly so that when resources update
+ * with new releases the built-in will update.
+ * @author Robert Field
+ */
+class Startup {
+
+ // Store one entry in the start-up list
+ private static class StartupEntry {
+
+ // is this a JShell built-in?
+ private final boolean isBuiltIn;
+
+ // the file or resource name
+ private final String name;
+
+ // the commands/snippets as text
+ private final String content;
+
+ // for files, the date/time read in -- makes clear it is a snapshot
+ private final String timeStamp;
+
+ StartupEntry(boolean isBuiltIn, String name, String content) {
+ this(isBuiltIn, name, content, "");
+ }
+
+ StartupEntry(boolean isBuiltIn, String name, String content, String timeStamp) {
+ this.isBuiltIn = isBuiltIn;
+ this.name = name;
+ this.content = content;
+ this.timeStamp = timeStamp;
+ }
+
+ // string form to store in storage (e.g. Preferences)
+ String storedForm() {
+ return (isBuiltIn ? "*" : "-") + RECORD_SEPARATOR +
+ name + RECORD_SEPARATOR +
+ timeStamp + RECORD_SEPARATOR +
+ content + RECORD_SEPARATOR;
+ }
+
+ // the content
+ @Override
+ public String toString() {
+ return content;
+ }
+
+ @Override
+ public int hashCode() {
+ int hash = 7;
+ hash = 41 * hash + (this.isBuiltIn ? 1 : 0);
+ hash = 41 * hash + Objects.hashCode(this.name);
+ if (!isBuiltIn) {
+ hash = 41 * hash + Objects.hashCode(this.content);
+ }
+ return hash;
+ }
+
+ // built-ins match on name only. Time stamp isn't considered
+ @Override
+ public boolean equals(Object o) {
+ if (!(o instanceof StartupEntry)) {
+ return false;
+ }
+ StartupEntry sue = (StartupEntry) o;
+ return isBuiltIn == sue.isBuiltIn &&
+ name.equals(sue.name) &&
+ (isBuiltIn || content.equals(sue.content));
+ }
+ }
+
+ private static final String DEFAULT_STARTUP_NAME = "DEFAULT";
+
+ // cached DEFAULT start-up
+ private static Startup defaultStartup = null;
+
+ // the list of entries
+ private List<StartupEntry> entries;
+
+ // the concatenated content of the list of entries
+ private String content;
+
+ // created only with factory methods (below)
+ private Startup(List<StartupEntry> entries) {
+ this.entries = entries;
+ this.content = entries.stream()
+ .map(sue -> sue.toString())
+ .collect(joining());
+ }
+
+ private Startup(StartupEntry entry) {
+ this(Collections.singletonList(entry));
+ }
+
+ // retrieve the content
+ @Override
+ public String toString() {
+ return content;
+ }
+
+ @Override
+ public int hashCode() {
+ return 9 + Objects.hashCode(this.entries);
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ return (o instanceof Startup)
+ && entries.equals(((Startup) o).entries);
+ }
+
+ // are there no entries ("-none")?
+ boolean isEmpty() {
+ return entries.isEmpty();
+ }
+
+ // is this the "-default" setting -- one entry which is DEFAULT
+ boolean isDefault() {
+ if (entries.size() == 1) {
+ StartupEntry sue = entries.get(0);
+ if (sue.isBuiltIn && sue.name.equals(DEFAULT_STARTUP_NAME)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ // string form to store in storage (e.g. Preferences)
+ String storedForm() {
+ return entries.stream()
+ .map(sue -> sue.storedForm())
+ .collect(joining());
+ }
+
+ // show commands to re-create
+ String show(boolean isRetained) {
+ String cmd = "/set start " + (isRetained ? "-retain " : "");
+ if (isDefault()) {
+ return cmd + "-default\n";
+ } else if (isEmpty()) {
+ return cmd + "-none\n";
+ } else {
+ return entries.stream()
+ .map(sue -> sue.name)
+ .collect(joining(" ", cmd, "\n"));
+ }
+ }
+
+ // show corresponding contents for show()
+ String showDetail() {
+ if (isDefault() || isEmpty()) {
+ return "";
+ } else {
+ return entries.stream()
+ .map(sue -> "---- " + sue.name
+ + (sue.timeStamp.isEmpty()
+ ? ""
+ : " @ " + sue.timeStamp)
+ + " ----\n" + sue.content)
+ .collect(joining());
+ }
+ }
+
+ /**
+ * Factory method: Unpack from stored form.
+ *
+ * @param storedForm the Startup in the form as stored on persistent
+ * storage (e.g. Preferences)
+ * @param mh handler for error messages
+ * @return Startup, or default startup when error (message has been printed)
+ */
+ static Startup unpack(String storedForm, MessageHandler mh) {
+ if (storedForm != null) {
+ if (storedForm.isEmpty()) {
+ return noStartup();
+ }
+ try {
+ String[] all = storedForm.split(RECORD_SEPARATOR);
+ if (all.length == 1) {
+ // legacy (content only)
+ return new Startup(new StartupEntry(false, "user.jsh", storedForm));
+ } else if (all.length % 4 == 0) {
+ List<StartupEntry> e = new ArrayList<>(all.length / 4);
+ for (int i = 0; i < all.length; i += 4) {
+ final boolean isBuiltIn;
+ switch (all[i]) {
+ case "*":
+ isBuiltIn = true;
+ break;
+ case "-":
+ isBuiltIn = false;
+ break;
+ default:
+ throw new IllegalArgumentException("Unexpected StartupEntry kind: " + all[i]);
+ }
+ String name = all[i + 1];
+ String timeStamp = all[i + 2];
+ String content = all[i + 3];
+ if (isBuiltIn) {
+ // update to current definition, use stored if removed/error
+ String resource = getResource(name);
+ if (resource != null) {
+ content = resource;
+ }
+ }
+ e.add(new StartupEntry(isBuiltIn, name, content, timeStamp));
+ }
+ return new Startup(e);
+ } else {
+ throw new IllegalArgumentException("Unexpected StartupEntry entry count: " + all.length);
+ }
+ } catch (Exception ex) {
+ mh.errormsg("jshell.err.corrupted.stored.startup", ex.getMessage());
+ }
+ }
+ return defaultStartup(mh);
+ }
+
+ /**
+ * Factory method: Read Startup from a list of external files or resources.
+ *
+ * @param fns list of file/resource names to access
+ * @param context printable non-natural language context for errors
+ * @param mh handler for error messages
+ * @return files as Startup, or null when error (message has been printed)
+ */
+ static Startup fromFileList(List<String> fns, String context, MessageHandler mh) {
+ List<StartupEntry> entries = fns.stream()
+ .map(fn -> readFile(fn, context, mh))
+ .collect(toList());
+ if (entries.stream().anyMatch(sue -> sue == null)) {
+ return null;
+ }
+ return new Startup(entries);
+ }
+
+ /**
+ * Read a external file or a resource.
+ *
+ * @param filename file/resource to access
+ * @param context printable non-natural language context for errors
+ * @param mh handler for error messages
+ * @return file as startup entry, or null when error (message has been printed)
+ */
+ private static StartupEntry readFile(String filename, String context, MessageHandler mh) {
+ if (filename != null) {
+ try {
+ byte[] encoded = Files.readAllBytes(toPathResolvingUserHome(filename));
+ return new StartupEntry(false, filename, new String(encoded),
+ LocalDateTime.now().format(DateTimeFormatter.ofLocalizedDateTime(FormatStyle.MEDIUM)));
+ } catch (AccessDeniedException e) {
+ mh.errormsg("jshell.err.file.not.accessible", context, filename, e.getMessage());
+ } catch (NoSuchFileException e) {
+ String resource = getResource(filename);
+ if (resource != null) {
+ // Not found as file, but found as resource
+ return new StartupEntry(true, filename, resource);
+ }
+ mh.errormsg("jshell.err.file.not.found", context, filename);
+ } catch (Exception e) {
+ mh.errormsg("jshell.err.file.exception", context, filename, e);
+ }
+ } else {
+ mh.errormsg("jshell.err.file.filename", context);
+ }
+ return null;
+
+ }
+
+ /**
+ * Factory method: The empty Startup ("-none").
+ *
+ * @return the empty Startup
+ */
+ static Startup noStartup() {
+ return new Startup(Collections.emptyList());
+ }
+
+ /**
+ * Factory method: The default Startup ("-default.").
+ *
+ * @param mh handler for error messages
+ * @return The default Startup, or empty startup when error (message has been printed)
+ */
+ static Startup defaultStartup(MessageHandler mh) {
+ if (defaultStartup != null) {
+ return defaultStartup;
+ }
+ try {
+ String content = readResource(DEFAULT_STARTUP_NAME);
+ return defaultStartup = new Startup(
+ new StartupEntry(true, DEFAULT_STARTUP_NAME, content));
+ } catch (AccessDeniedException e) {
+ mh.errormsg("jshell.err.file.not.accessible", "jshell", DEFAULT_STARTUP_NAME, e.getMessage());
+ } catch (NoSuchFileException e) {
+ mh.errormsg("jshell.err.file.not.found", "jshell", DEFAULT_STARTUP_NAME);
+ } catch (Exception e) {
+ mh.errormsg("jshell.err.file.exception", "jshell", DEFAULT_STARTUP_NAME, e);
+ }
+ return defaultStartup = noStartup();
+ }
+
+}
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties Thu Jan 19 07:02:34 2017 -0800
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties Thu Jan 19 11:12:02 2017 -0800
@@ -148,6 +148,8 @@
jshell.err.the.snippet.cannot.be.used.with.this.command = This command does not accept the snippet ''{0}'' : {1}
jshell.err.retained.mode.failure = Failure in retained modes (modes cleared) -- {0} {1}
+jshell.err.corrupted.stored.startup = Corrupted stored startup, using default -- {0}
+
jshell.console.see.more = <press tab to see more>
jshell.console.see.javadoc = <press shift-tab again to see javadoc>
jshell.console.see.help = <press shift-tab again to see detailed help>
--- a/langtools/src/jdk.jshell/share/classes/jdk/jshell/tool/resources/DEFAULT.jsh Thu Jan 19 07:02:34 2017 -0800
+++ b/langtools/src/jdk.jshell/share/classes/jdk/jshell/tool/resources/DEFAULT.jsh Thu Jan 19 11:12:02 2017 -0800
@@ -8,4 +8,3 @@
import java.util.prefs.*;
import java.util.regex.*;
import java.util.stream.*;
-
--- a/langtools/test/jdk/jshell/ToolCommandOptionTest.java Thu Jan 19 07:02:34 2017 -0800
+++ b/langtools/test/jdk/jshell/ToolCommandOptionTest.java Thu Jan 19 11:12:02 2017 -0800
@@ -23,7 +23,7 @@
/*
* @test
- * @bug 8157395 8157393 8157517 8158738 8167128 8163840 8167637 8170368 8172102
+ * @bug 8157395 8157393 8157517 8158738 8167128 8163840 8167637 8170368 8172102 8172179
* @summary Tests of jshell comand options, and undoing operations
* @modules jdk.jshell/jdk.internal.jshell.tool
* jdk.compiler/com.sun.tools.javac.api
@@ -273,14 +273,13 @@
(a) -> assertCommand(a, "/set start DEFAULT PRINTING",
""),
(a) -> assertCommandOutputContains(a, "/set start",
- "void println", "import java.util.*"),
+ "/set start DEFAULT PRINTING", "void println", "import java.util.*"),
(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) -> assertCommandOutputContains(a, "/set start",
+ "| /set start " + startup + "\n" +
+ "| ---- " + startup + " @ ", " ----\n" +
+ "| int iAmHere = 1234;\n"),
(a) -> assertCommand(a, "/se sta -no",
""),
(a) -> assertCommand(a, "/set start",
@@ -322,11 +321,18 @@
"| /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")
+ (a) -> assertCommand(a, "/set start DEFAULT PRINTING",
+ ""),
+ (a) -> assertCommandOutputStartsWith(a, "/set start",
+ "| /set start -retain " + startup.toString() + "\n" +
+ "| /set start DEFAULT PRINTING\n" +
+ "| ---- " + startup.toString() + " @ "),
+ (a) -> assertCommandOutputContains(a, "/set start",
+ "| ---- DEFAULT ----\n",
+ "| ---- PRINTING ----\n",
+ "| int iAmHere = 1234;\n",
+ "| void println(String s)",
+ "| import java.io.*;")
);
}