Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jline3 and other console improvements #935

Open
wants to merge 46 commits into
base: dev
Choose a base branch
from
Open

Jline3 and other console improvements #935

wants to merge 46 commits into from

Conversation

mastercoms
Copy link
Member

@mastercoms mastercoms commented Jun 1, 2018

This moves Glowstone to jline3. It comes with a better formatted console, compatibility improvements and more.

In addition to jline3, there are also new console manager tasks in addition to the already present command task: evaluation tasks and console tasks. EvalTasks parse Java code from the console and ConsoleTasks add new ways to customize the console through binds (not implemented), settings, keymaps and widgets.

Co-authored-by: Pr0methean [email protected]

@NonNls private static String CONSOLE_DATE = "HH:mm:ss";
@NonNls private static String FILE_DATE = "yyyy/MM/dd HH:mm:ss";
@NonNls private static String CONSOLE_PROMPT = ">";
@NonNls private static String CONSOLE_PROMPT = "> "; // TODO: fix prompt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the problem here? Document in detail or link a bug.

addReplacement(ChatColor.RESET, "\u001B[39;0m"); // NON-NLS
}

private static void addReplacement(ChatColor formatting, String ansi) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make Ansi @NonNls, and then the // NON-NLS comments probably won't be needed.

this.level = level;
}
if (record.getThrown() != null) {
// StringWriter's close() is trivial
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use try-with-resources anyway, for future-proofing. It's probably equally concise.

for (ChatColor color : colors) {
if (this.color && replacements.containsKey(color)) {
string = string.replaceAll("(?i)" + color, replacements.get(color));
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use the ternary operator here.

}
candidates.addAll(completions);
completions = server.getScheduler().syncIfNeeded(() -> server.getCommandMap()
.tabComplete(sender, line.line()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the lambda on one line.

if (task.call()) {
try {
Object returned = MethodInvocationUtils
.invokeStaticMethod(getCompiledClass("REPLShell"), "run",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the methods of MethodInvocationUtils to this class, unless and until another class uses them.

+ '"')); // NON-NLS
if (command.startsWith("$")) { // NON-NLS
server.getScheduler().runTask(null,
new EvalTask(command.substring(1), command.startsWith("$$")));

This comment was marked as outdated.

@@ -0,0 +1,14 @@
package net.glowstone.i18n;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just add these methods to LoggableLocalizedString instead?

import java.util.logging.Level;
import javax.tools.FileObject;
import javax.tools.ForwardingJavaFileManager;
import javax.tools.JavaCompiler;

This comment was marked as outdated.

}
}

public class ClassDataOutputStream extends OutputStream {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do differently than the underlying ByteArrayOutputStream? If this can't simply be replaced by a BAOS, document why not, and can we use a FilterOutputStream instead?

@Pr0methean
Copy link
Contributor

@mastercoms asked me to commit the changes I could. I've done that; the REPL implementation is now on branch jline3repl.

@VaiTon
Copy link
Contributor

VaiTon commented Feb 10, 2019

Why is this stalled?

@mastercoms
Copy link
Member Author

Unfortunately, we encountered regressions in the terminal on Windows. I haven't tested the latest version to see if it was fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants