You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by vk...@apache.org on 2021/02/02 01:01:01 UTC
[ignite-3] branch main updated: IGNITE-13947 - Fixed progress bar
rendering according to terminal width (#42)
This is an automated email from the ASF dual-hosted git repository.
vkulichenko pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 77d8fb5 IGNITE-13947 - Fixed progress bar rendering according to terminal width (#42)
77d8fb5 is described below
commit 77d8fb51669bffd76de5313b7dc9da5284b9d971
Author: Kirill Gusakov <kg...@gmail.com>
AuthorDate: Tue Feb 2 04:00:51 2021 +0300
IGNITE-13947 - Fixed progress bar rendering according to terminal width (#42)
* Fix progressbar
* IGNITE-13947 Fixed progress bar rendering according to terminal width
* Fix checkstyle
* Fix codestyle
* Fixed terminal and applicationContext lifecycle in tests
* Fixed checkstyle
---
.../org/apache/ignite/cli/InteractiveWrapper.java | 70 +++++++++++----------
.../cli/builtins/module/MavenArtifactResolver.java | 12 +++-
.../ignite/cli/builtins/node/NodeManager.java | 12 +++-
.../org/apache/ignite/cli/spec/IgniteCliSpec.java | 7 ++-
.../ProgressBar.java} | 73 ++++++++++++++--------
.../org/apache/ignite/cli/ui/TerminalFactory.java | 27 ++++++++
.../apache/ignite/cli/IgniteCliInterfaceTest.java | 7 +++
.../org/apache/ignite/cli/ui/ProgressBarTest.java | 63 +++++++++++++++++++
8 files changed, 206 insertions(+), 65 deletions(-)
diff --git a/modules/cli/src/main/java/org/apache/ignite/cli/InteractiveWrapper.java b/modules/cli/src/main/java/org/apache/ignite/cli/InteractiveWrapper.java
index 651aec3..6e2d876 100644
--- a/modules/cli/src/main/java/org/apache/ignite/cli/InteractiveWrapper.java
+++ b/modules/cli/src/main/java/org/apache/ignite/cli/InteractiveWrapper.java
@@ -17,7 +17,6 @@
package org.apache.ignite.cli;
-import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import org.jline.console.SystemRegistry;
@@ -33,7 +32,6 @@ import org.jline.reader.Reference;
import org.jline.reader.UserInterruptException;
import org.jline.reader.impl.DefaultParser;
import org.jline.terminal.Terminal;
-import org.jline.terminal.TerminalBuilder;
import org.jline.widget.TailTipWidgets;
import picocli.CommandLine;
import picocli.shell.jline3.PicocliCommands;
@@ -42,6 +40,16 @@ import picocli.shell.jline3.PicocliCommands;
* Interactive shell mode for Ignite CLI.
*/
public class InteractiveWrapper {
+ /** System terminal instance. */
+ private final Terminal terminal;
+
+ /**
+ * @param terminal Terminal.
+ */
+ public InteractiveWrapper(Terminal terminal) {
+ this.terminal = terminal;
+ }
+
/**
* Start interactive shell.
*
@@ -55,46 +63,42 @@ public class InteractiveWrapper {
};
Parser parser = new DefaultParser();
- try (Terminal terminal = TerminalBuilder.builder().build()) {
- SystemRegistry sysRegistry = new SystemRegistryImpl(parser, terminal, InteractiveWrapper::workDir, null);
- sysRegistry.setCommandRegistries(picocliCommands);
- LineReader reader = LineReaderBuilder.builder()
- .terminal(terminal)
- .completer(sysRegistry.completer())
- .parser(parser)
- .variable(LineReader.LIST_MAX, 50) // max tab completion candidates
- .build();
+ SystemRegistry sysRegistry = new SystemRegistryImpl(parser, terminal, InteractiveWrapper::workDir, null);
+ sysRegistry.setCommandRegistries(picocliCommands);
+
+ LineReader reader = LineReaderBuilder.builder()
+ .terminal(terminal)
+ .completer(sysRegistry.completer())
+ .parser(parser)
+ .variable(LineReader.LIST_MAX, 50) // max tab completion candidates
+ .build();
- TailTipWidgets widgets = new TailTipWidgets(reader, sysRegistry::commandDescription, 5, TailTipWidgets.TipType.COMPLETER);
- widgets.enable();
+ TailTipWidgets widgets = new TailTipWidgets(reader, sysRegistry::commandDescription, 5, TailTipWidgets.TipType.COMPLETER);
+ widgets.enable();
- KeyMap<Binding> keyMap = reader.getKeyMaps().get("main");
- keyMap.bind(new Reference("tailtip-toggle"), KeyMap.alt("s"));
+ KeyMap<Binding> keyMap = reader.getKeyMaps().get("main");
+ keyMap.bind(new Reference("tailtip-toggle"), KeyMap.alt("s"));
- String prompt = "ignite> ";
- String rightPrompt = null;
+ String prompt = "ignite> ";
+ String rightPrompt = null;
- String line;
- while (true) {
- try {
- sysRegistry.cleanUp();
+ String line;
+ while (true) {
+ try {
+ sysRegistry.cleanUp();
- line = reader.readLine(prompt, rightPrompt, (MaskingCallback) null, null);
+ line = reader.readLine(prompt, rightPrompt, (MaskingCallback) null, null);
- sysRegistry.execute(line);
- } catch (UserInterruptException ignored) {
- // Ignore
- } catch (EndOfFileException e) {
- return;
- } catch (Exception e) {
- sysRegistry.trace(e);
- }
+ sysRegistry.execute(line);
+ } catch (UserInterruptException ignored) {
+ // Ignore
+ } catch (EndOfFileException e) {
+ return;
+ } catch (Exception e) {
+ sysRegistry.trace(e);
}
}
- catch (IOException e) {
- e.printStackTrace();
- }
}
/** */
diff --git a/modules/cli/src/main/java/org/apache/ignite/cli/builtins/module/MavenArtifactResolver.java b/modules/cli/src/main/java/org/apache/ignite/cli/builtins/module/MavenArtifactResolver.java
index fc2daf0..384185f 100644
--- a/modules/cli/src/main/java/org/apache/ignite/cli/builtins/module/MavenArtifactResolver.java
+++ b/modules/cli/src/main/java/org/apache/ignite/cli/builtins/module/MavenArtifactResolver.java
@@ -29,7 +29,7 @@ import java.util.stream.Collectors;
import javax.inject.Inject;
import javax.inject.Singleton;
import org.apache.ignite.cli.IgniteCLIException;
-import org.apache.ignite.cli.IgniteProgressBar;
+import org.apache.ignite.cli.ui.ProgressBar;
import org.apache.ignite.cli.builtins.SystemPathResolver;
import org.apache.ivy.Ivy;
import org.apache.ivy.core.IvyContext;
@@ -51,6 +51,7 @@ import org.apache.ivy.plugins.resolver.ChainResolver;
import org.apache.ivy.plugins.resolver.IBiblioResolver;
import org.apache.ivy.util.AbstractMessageLogger;
import org.apache.ivy.util.Message;
+import org.jline.terminal.Terminal;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -65,6 +66,9 @@ public class MavenArtifactResolver {
/** Resolver of system paths. **/
private final SystemPathResolver pathRslvr;
+ /** System terminal instance, needed for receiving info about terminal settings. **/
+ private final Terminal terminal;
+
/** Console writer for output user messages. **/
private PrintWriter out;
@@ -72,10 +76,12 @@ public class MavenArtifactResolver {
* Creates resolver
*
* @param pathRslvr Resolver of system paths like home directory and etc.
+ * @param terminal User system terminal.
*/
@Inject
- public MavenArtifactResolver(SystemPathResolver pathRslvr) {
+ public MavenArtifactResolver(SystemPathResolver pathRslvr, Terminal terminal) {
this.pathRslvr = pathRslvr;
+ this.terminal = terminal;
}
/**
@@ -110,7 +116,7 @@ public class MavenArtifactResolver {
out.println("Installing " + String.join(":", grpId, artifactId, ver) + "...");
- try (IgniteProgressBar bar = new IgniteProgressBar(out, 100)) {
+ try (ProgressBar bar = new ProgressBar(out, 100, terminal.getWidth())) {
ivy.getEventManager().addIvyListener(event -> {
if (event instanceof EndResolveEvent) {
int cnt = ((EndResolveEvent)event).getReport().getArtifacts().size();
diff --git a/modules/cli/src/main/java/org/apache/ignite/cli/builtins/node/NodeManager.java b/modules/cli/src/main/java/org/apache/ignite/cli/builtins/node/NodeManager.java
index fb662c1..05f5f37 100644
--- a/modules/cli/src/main/java/org/apache/ignite/cli/builtins/node/NodeManager.java
+++ b/modules/cli/src/main/java/org/apache/ignite/cli/builtins/node/NodeManager.java
@@ -32,8 +32,9 @@ import java.util.stream.Stream;
import javax.inject.Inject;
import javax.inject.Singleton;
import org.apache.ignite.cli.IgniteCLIException;
-import org.apache.ignite.cli.IgniteProgressBar;
+import org.apache.ignite.cli.ui.ProgressBar;
import org.apache.ignite.cli.builtins.module.ModuleRegistry;
+import org.jline.terminal.Terminal;
/**
* Manager of local Ignite nodes.
@@ -52,14 +53,19 @@ public class NodeManager {
/** Module registry. **/
private final ModuleRegistry moduleRegistry;
+ /** System terminal. **/
+ private final Terminal terminal;
+
/**
* Creates node manager.
*
* @param moduleRegistry Module registry.
+ * @param terminal System terminal instance.
*/
@Inject
- public NodeManager(ModuleRegistry moduleRegistry) {
+ public NodeManager(ModuleRegistry moduleRegistry, Terminal terminal) {
this.moduleRegistry = moduleRegistry;
+ this.terminal = terminal;
}
/**
@@ -106,7 +112,7 @@ public class NodeManager {
Process p = pb.start();
- try (var bar = new IgniteProgressBar(out, 100)) {
+ try (var bar = new ProgressBar(out, 100, terminal.getWidth())) {
bar.stepPeriodically(300);
if (!waitForStart("Apache Ignite started successfully!", logFile, NODE_START_TIMEOUT)) {
diff --git a/modules/cli/src/main/java/org/apache/ignite/cli/spec/IgniteCliSpec.java b/modules/cli/src/main/java/org/apache/ignite/cli/spec/IgniteCliSpec.java
index a41d17d..9f67946 100644
--- a/modules/cli/src/main/java/org/apache/ignite/cli/spec/IgniteCliSpec.java
+++ b/modules/cli/src/main/java/org/apache/ignite/cli/spec/IgniteCliSpec.java
@@ -24,6 +24,7 @@ import java.nio.file.Path;
import java.util.List;
import java.util.ServiceLoader;
import java.util.stream.Collectors;
+import javax.inject.Inject;
import io.micronaut.context.ApplicationContext;
import org.apache.ignite.cli.CliPathsConfigLoader;
import org.apache.ignite.cli.CommandFactory;
@@ -33,6 +34,7 @@ import org.apache.ignite.cli.IgniteCLIException;
import org.apache.ignite.cli.InteractiveWrapper;
import org.apache.ignite.cli.builtins.module.ModuleRegistry;
import org.apache.ignite.cli.common.IgniteCommand;
+import org.jline.terminal.Terminal;
import picocli.CommandLine;
/**
@@ -53,6 +55,9 @@ public class IgniteCliSpec extends CommandSpec {
@CommandLine.Option(names = "-i", hidden = true, required = false)
private boolean interactive;
+ @Inject
+ private Terminal terminal;
+
/** {@inheritDoc} */
@Override public void run() {
CommandLine cli = spec.commandLine();
@@ -60,7 +65,7 @@ public class IgniteCliSpec extends CommandSpec {
cli.getOut().print(banner());
if (interactive)
- new InteractiveWrapper().run(cli);
+ new InteractiveWrapper(terminal).run(cli);
else
cli.usage(cli.getOut());
}
diff --git a/modules/cli/src/main/java/org/apache/ignite/cli/IgniteProgressBar.java b/modules/cli/src/main/java/org/apache/ignite/cli/ui/ProgressBar.java
similarity index 67%
rename from modules/cli/src/main/java/org/apache/ignite/cli/IgniteProgressBar.java
rename to modules/cli/src/main/java/org/apache/ignite/cli/ui/ProgressBar.java
index b27ebba..c7dc044 100644
--- a/modules/cli/src/main/java/org/apache/ignite/cli/IgniteProgressBar.java
+++ b/modules/cli/src/main/java/org/apache/ignite/cli/ui/ProgressBar.java
@@ -15,18 +15,23 @@
* limitations under the License.
*/
-package org.apache.ignite.cli;
+package org.apache.ignite.cli.ui;
import java.io.PrintWriter;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import picocli.CommandLine.Help.Ansi;
/**
* Basic implementation of a progress bar.
*/
-public class IgniteProgressBar implements AutoCloseable {
+public class ProgressBar implements AutoCloseable {
+ /** Logger. **/
+ private final Logger log = LoggerFactory.getLogger(getClass());
+
/** Out to output the progress bar UI.. */
private final PrintWriter out;
@@ -36,20 +41,40 @@ public class IgniteProgressBar implements AutoCloseable {
/** Maximum progress bar value. */
private int max;
+ /** Target width of bar in symbols. */
+ private final int targetBarWidth;
+
/** Execute. */
private ScheduledExecutorService exec;
/**
* Creates a new progress bar.
*
+ * @param out Output which terminal render will use
* @param initMax Initial maximum number of steps.
+ * @param terminalWidth Width of user terminal for scale progress bar length
*/
- public IgniteProgressBar(PrintWriter out, int initMax) {
+ public ProgressBar(PrintWriter out, int initMax, int terminalWidth) {
this.out = out;
assert initMax > 0;
max = initMax;
+
+ // A huge progress bar for big terminals looks ugly.
+ // It's better to have just enough wide progress bar.
+ targetBarWidth = Math.min(100, terminalWidth);
+ }
+
+ /**
+ * Updates maximum number of steps.
+ *
+ * @param newMax New maximum.
+ */
+ public void setMax(int newMax) {
+ assert newMax > 0;
+
+ max = newMax;
}
/**
@@ -76,17 +101,6 @@ public class IgniteProgressBar implements AutoCloseable {
}
}
- /**
- * Updates maximum number of steps.
- *
- * @param newMax New maximum.
- */
- public void setMax(int newMax) {
- assert newMax > 0;
-
- max = newMax;
- }
-
/** {@inheritDoc} */
@Override public void close() {
while (curr < max) {
@@ -111,27 +125,36 @@ public class IgniteProgressBar implements AutoCloseable {
private String render() {
assert curr <= max;
- int completed = (int)((double)curr / (double)max * 100);
+ var completedPart = ((double)curr / (double)max);
+
+ // Space reserved for '||Done!'
+ var reservedSpace = 7;
+
+ if (targetBarWidth < reservedSpace) {
+ log.warn("Terminal width is so small to show the progress bar");
+
+ return "";
+ }
+
+ var numOfCompletedSymbols = (int) (completedPart * (targetBarWidth - reservedSpace));
StringBuilder sb = new StringBuilder("|");
- sb.append("=".repeat(completed));
+ sb.append("=".repeat(numOfCompletedSymbols));
String percentage;
int percentageLen;
- if (completed < 100) {
- sb.append('>').append(" ".repeat(99 - completed));
+ if (completedPart < 1) {
+ sb.append('>').append(" ".repeat(targetBarWidth - reservedSpace - numOfCompletedSymbols));
- percentage = completed + "%";
+ percentage = (int) (completedPart * 100) + "%";
percentageLen = percentage.length();
- }
- else {
- percentage = "@|green,bold Done!|@";
- percentageLen = 5;
- }
- sb.append("|").append(" ".repeat(6 - percentageLen)).append(percentage);
+ sb.append("|").append(" ".repeat(4 - percentageLen)).append(percentage);
+ }
+ else
+ sb.append("=|@|green,bold Done!|@");
return Ansi.AUTO.string(sb.toString());
}
diff --git a/modules/cli/src/main/java/org/apache/ignite/cli/ui/TerminalFactory.java b/modules/cli/src/main/java/org/apache/ignite/cli/ui/TerminalFactory.java
new file mode 100644
index 0000000..522d5b3
--- /dev/null
+++ b/modules/cli/src/main/java/org/apache/ignite/cli/ui/TerminalFactory.java
@@ -0,0 +1,27 @@
+package org.apache.ignite.cli.ui;
+
+import java.io.IOException;
+import javax.inject.Singleton;
+import io.micronaut.context.annotation.Bean;
+import io.micronaut.context.annotation.Factory;
+import org.jline.terminal.Terminal;
+import org.jline.terminal.TerminalBuilder;
+
+/**
+ * Factory for producing JLine {@link Terminal} instances
+ */
+@Factory
+public class TerminalFactory {
+ /**
+ * Produce terminal instances.
+ *
+ * Important: It's always must be a singleton bean.
+ * JLine has an issues with building more than 1 terminal instance per process
+ * @return Terminal instance
+ */
+ @Bean(preDestroy = "close")
+ @Singleton
+ public Terminal terminal() throws IOException {
+ return TerminalBuilder.terminal();
+ }
+}
diff --git a/modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java b/modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
index 5775157..45790c5 100644
--- a/modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
+++ b/modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
@@ -37,6 +37,8 @@ import org.apache.ignite.cli.builtins.module.ModuleRegistry;
import org.apache.ignite.cli.builtins.module.StandardModuleDefinition;
import org.apache.ignite.cli.builtins.node.NodeManager;
import org.apache.ignite.cli.spec.IgniteCliSpec;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
@@ -87,6 +89,11 @@ public class IgniteCliInterfaceTest {
out = new ByteArrayOutputStream();
}
+ @AfterEach
+ private void tearDown() {
+ applicationCtx.stop();
+ }
+
/** */
CommandLine commandLine(ApplicationContext applicationCtx) {
CommandLine.IFactory factory = new CommandFactory(applicationCtx);
diff --git a/modules/cli/src/test/java/org/apache/ignite/cli/ui/ProgressBarTest.java b/modules/cli/src/test/java/org/apache/ignite/cli/ui/ProgressBarTest.java
new file mode 100644
index 0000000..66b4021
--- /dev/null
+++ b/modules/cli/src/test/java/org/apache/ignite/cli/ui/ProgressBarTest.java
@@ -0,0 +1,63 @@
+package org.apache.ignite.cli.ui;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintWriter;
+import org.jline.terminal.TerminalBuilder;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/** */
+public class ProgressBarTest {
+ /** */
+ private PrintWriter out;
+
+ /** */
+ private ByteArrayOutputStream outputStream;
+
+ /** */
+ @BeforeEach
+ private void setUp() {
+ outputStream = new ByteArrayOutputStream();
+ out = new PrintWriter(outputStream, true);
+ }
+
+ /** */
+ @AfterEach
+ private void tearDown() throws IOException {
+ out.close();
+ outputStream.close();
+ }
+
+ /** */
+ @Test
+ public void testScaledToTerminalWidth() throws IOException {
+ var progressBar = new ProgressBar(out, 3, 80);
+ progressBar.step();
+ assertEquals(80, outputStream.toString().replace("\r", "").length());
+ assertEquals(
+ "\r|========================> | 33%",
+ outputStream.toString()
+ );
+ }
+
+ /** */
+ @Test
+ public void testRedundantStepsProgressBar() {
+ var progressBar = new ProgressBar(out, 3, 80);
+ progressBar.step();
+ progressBar.step();
+ progressBar.step();
+ progressBar.step();
+ assertEquals(
+ "\r|========================> | 33%" +
+ "\r|================================================> | 66%" +
+ "\r|==========================================================================|Done!" +
+ "\r|==========================================================================|Done!",
+ outputStream.toString()
+ );
+ }
+}