From f64ec5a77f885ec7d495b5f8fca1f2b396a2b41f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=BCger?= Date: Fri, 7 Dec 2018 13:59:27 +0100 Subject: [PATCH] KARAF-6038 Several improvements of Shell Table code * Optimizing StringUtil.repeat * Color and bold handling * Get rid of one StringBuilder in Row.getContent * Minor improvements of ShellTable.getEncoding (+ big FIXME) All test cases still run. --- .../help/CommandListHelpProvider.java | 10 +-- .../apache/karaf/shell/support/table/Col.java | 80 +++++-------------- .../apache/karaf/shell/support/table/Row.java | 19 +++-- .../karaf/shell/support/table/ShellTable.java | 66 ++++++++------- .../karaf/shell/support/table/StringUtil.java | 16 ++-- 5 files changed, 75 insertions(+), 116 deletions(-) diff --git a/shell/core/src/main/java/org/apache/karaf/shell/impl/console/commands/help/CommandListHelpProvider.java b/shell/core/src/main/java/org/apache/karaf/shell/impl/console/commands/help/CommandListHelpProvider.java index 857f56e6d0..5ae590077f 100644 --- a/shell/core/src/main/java/org/apache/karaf/shell/impl/console/commands/help/CommandListHelpProvider.java +++ b/shell/core/src/main/java/org/apache/karaf/shell/impl/console/commands/help/CommandListHelpProvider.java @@ -27,13 +27,14 @@ import java.util.Map; import java.util.Set; import java.util.SortedMap; import java.util.TreeMap; - import org.apache.karaf.shell.api.console.Command; import org.apache.karaf.shell.api.console.Session; import org.apache.karaf.shell.api.console.Terminal; import org.apache.karaf.shell.support.NameScoping; +import org.apache.karaf.shell.support.ansi.SimpleAnsi; import org.apache.karaf.shell.support.table.Col; import org.apache.karaf.shell.support.table.ShellTable; +import org.apache.karaf.shell.support.table.StringUtil; public class CommandListHelpProvider implements HelpProvider { @@ -143,7 +144,7 @@ public class CommandListHelpProvider implements HelpProvider { table.column(new Col("")); } if (cyan) { - col.cyan(); + col.colorProvider(s -> SimpleAnsi.COLOR_CYAN); } else { col.bold(); } @@ -151,10 +152,7 @@ public class CommandListHelpProvider implements HelpProvider { table.column(new Col("Description").wrap()); for (Map.Entry entry : commands.entrySet()) { String key = NameScoping.getCommandNameWithoutGlobalPrefix(session, entry.getKey()); - String prefix = ""; - for (int i = 0; i < indent; i++) { - prefix += " "; - } + String prefix = StringUtil.repeat(" ", indent); if (list) { prefix += " *"; } diff --git a/shell/core/src/main/java/org/apache/karaf/shell/support/table/Col.java b/shell/core/src/main/java/org/apache/karaf/shell/support/table/Col.java index 5476e8988f..b52ce39c68 100644 --- a/shell/core/src/main/java/org/apache/karaf/shell/support/table/Col.java +++ b/shell/core/src/main/java/org/apache/karaf/shell/support/table/Col.java @@ -17,23 +17,20 @@ package org.apache.karaf.shell.support.table; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.List; +import java.util.Optional; import java.util.function.Function; import java.util.regex.Matcher; import java.util.regex.Pattern; - +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.apache.karaf.shell.support.ansi.SimpleAnsi; /** * Column definition. */ public class Col { - // This is kept here only for backwards compatibility - // and is used in cyan(boolean) method - private static final Function COLOR_CYAN = - (cellContent) -> SimpleAnsi.COLOR_CYAN; - - Function colorProvider; /** * Column header. @@ -49,9 +46,11 @@ public class Col { int size = 0; boolean wrap; - boolean bold; - boolean cyan; + private Function colorProvider; + + private String boldStringStart = ""; + private String boldStringEnd = ""; /** * Alignment @@ -101,32 +100,18 @@ public class Col { } public Col bold(boolean bold) { - this.bold = bold; + boldStringStart = bold ? SimpleAnsi.INTENSITY_BOLD : ""; + boldStringEnd = bold ? SimpleAnsi.INTENSITY_NORMAL : ""; return this; } - public Col cyan() { - return cyan(true); - } - - public Col cyan(boolean cyan) { - if(cyan) - colorProvider(COLOR_CYAN); - - // Only remove colorProvider if argument is false and - // member equals COLOR_CYAN - else if(this.colorProvider == COLOR_CYAN) - colorProvider(null); - - return this; - } - public int getSize() { return size; } public Col colorProvider(Function colorProvider) { - this.colorProvider = colorProvider; + // We need to wrap the provided provider again because for normal color it could return null. + this.colorProvider = content -> Optional.ofNullable(colorProvider.apply(content)).orElse(""); return this; } @@ -158,40 +143,17 @@ public class Col { } String getContent(String content) { - List lines = new ArrayList<>(); - lines.addAll(Arrays.asList(content.split("\n"))); - if (wrap) { - List wrapped = new ArrayList<>(); - for (String line : lines) { - wrapped.addAll(wrap(line)); - } - lines = wrapped; - } + String colorStringStart = colorProvider != null ? colorProvider.apply(content) : ""; + String colorStringEnd = colorProvider != null ? (!colorStringStart.isEmpty() ? SimpleAnsi.COLOR_DEFAULT : "") : ""; - String color = null; - if(colorProvider != null) { - color = colorProvider.apply(content); + Stream lines = Arrays.stream(content.split("\n")); + if (wrap) { + lines = lines.map(this::wrap).flatMap(Collection::stream); } - StringBuilder sb = new StringBuilder(); - for (String line : lines) { - if (sb.length() > 0) { - sb.append("\n"); - } - line = this.align.position(cut(line, size), this.size); - if (bold) { - line = SimpleAnsi.INTENSITY_BOLD + line + SimpleAnsi.INTENSITY_NORMAL; - } - - if(color != null) - sb.append(color); - - sb.append(line); - - if(color != null) - sb.append(SimpleAnsi.COLOR_DEFAULT); - } - return sb.toString(); + return lines + .map(line -> align.position(cut(line, size), size)) + .collect(Collectors.joining("\n", colorStringStart + boldStringStart, boldStringEnd + colorStringEnd)); } protected String cut(String content, int size) { @@ -225,6 +187,4 @@ public class Col { } return result; } - - } diff --git a/shell/core/src/main/java/org/apache/karaf/shell/support/table/Row.java b/shell/core/src/main/java/org/apache/karaf/shell/support/table/Row.java index 8a8dbd6963..38b615f7e8 100644 --- a/shell/core/src/main/java/org/apache/karaf/shell/support/table/Row.java +++ b/shell/core/src/main/java/org/apache/karaf/shell/support/table/Row.java @@ -53,13 +53,14 @@ public class Row { } String getContent(List cols, String separator) { - if (cols.size() != content.size()) { + int colSize = cols.size(); + if (colSize != content.size()) { throw new RuntimeException("Number of columns and number of content elements do not match"); } List contents = new ArrayList<>(); int lines = 0; - for (int col = 0; col < cols.size(); col++) { + for (int col = 0; col < colSize; col++) { String[] cnt = cols.get(col).getContent(content.get(col)).split("\n"); lines = Math.max(lines, cnt.length); contents.add(cnt); @@ -69,22 +70,20 @@ public class Row { if (line > 0) { st.append("\n"); } - StringBuilder st2 = new StringBuilder(); - for (int col = 0; col < cols.size(); col++) { + for (int col = 0; col < colSize; col++) { String[] strings = contents.get(col); if (col > 0) { - st2.append(separator); + st.append(separator); } if (line < strings.length) { - st2.append(strings[line]); + st.append(strings[line]); } else { - st2.append(StringUtil.repeat(" ", cols.get(col).getSize())); + st.append(StringUtil.repeat(" ", cols.get(col).getSize())); } } - while (st2.charAt(st2.length() - 1) == ' ') { - st2.setLength(st2.length() - 1); + while (st.charAt(st.length() - 1) == ' ') { + st.setLength(st.length() - 1); } - st.append(st2); } return st.toString(); } diff --git a/shell/core/src/main/java/org/apache/karaf/shell/support/table/ShellTable.java b/shell/core/src/main/java/org/apache/karaf/shell/support/table/ShellTable.java index 8235692aed..61770668f2 100644 --- a/shell/core/src/main/java/org/apache/karaf/shell/support/table/ShellTable.java +++ b/shell/core/src/main/java/org/apache/karaf/shell/support/table/ShellTable.java @@ -16,10 +16,6 @@ */ package org.apache.karaf.shell.support.table; -import org.apache.felix.gogo.runtime.threadio.ThreadPrintStream; -import org.apache.felix.service.command.Job; -import org.jline.terminal.Terminal; - import java.io.OutputStreamWriter; import java.io.PrintStream; import java.lang.reflect.Field; @@ -28,6 +24,9 @@ import java.nio.charset.CharsetEncoder; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.apache.felix.gogo.runtime.threadio.ThreadPrintStream; +import org.apache.felix.service.command.Job; +import org.jline.terminal.Terminal; public class ShellTable { @@ -52,7 +51,6 @@ public class ShellTable { private boolean forceAscii; public ShellTable() { - } public ShellTable noHeaders() { @@ -86,7 +84,7 @@ public class ShellTable { rows.add(row); return row; } - + public ShellTable forceAscii() { forceAscii = true; return this; @@ -107,11 +105,11 @@ public class ShellTable { print(out, true); } - public void print(PrintStream out, boolean format) { + public void print(PrintStream out, boolean format) { print(out, null, format); } - public void print(PrintStream out, Charset charset, boolean format) { + public void print(PrintStream out, Charset charset, boolean format) { boolean unicode = supportsUnicode(out, charset); String separator = unicode ? this.separator : DEFAULT_SEPARATOR_ASCII; @@ -131,7 +129,7 @@ public class ShellTable { out.println(headerLine); int iCol = 0; for (Col col : cols) { - if (iCol++ == 0) { + if (iCol == 0) { out.print(underline(col.getSize(), false, unicode)); } else { out.print(underline(col.getSize() + 3, true, unicode)); @@ -141,18 +139,22 @@ public class ShellTable { out.println(); } - for (Row row : rows) { - if (!format) { - if (separator == null || separator.equals(DEFAULT_SEPARATOR)) - out.println(row.getContent(cols, DEFAULT_SEPARATOR_NO_FORMAT)); - else out.println(row.getContent(cols, separator)); - } else { - out.println(row.getContent(cols, separator)); + if (format && rows.size() == 0) { + if (emptyTableText != null) { + out.println(emptyTableText); + } + } else { + for (Row row : rows) { + if (!format) { + if (separator == null || separator.equals(DEFAULT_SEPARATOR)) { + out.println(row.getContent(cols, DEFAULT_SEPARATOR_NO_FORMAT)); + } else { + out.println(row.getContent(cols, separator)); + } + } else { + out.println(row.getContent(cols, separator)); + } } - } - - if (format && rows.size() == 0 && emptyTableText != null) { - out.println(emptyTableText); } } @@ -167,24 +169,30 @@ public class ShellTable { return false; } CharsetEncoder encoder = charset.newEncoder(); - return encoder.canEncode(separator) - && encoder.canEncode(SEP_HORIZONTAL) - && encoder.canEncode(SEP_CROSS); + return encoder.canEncode(separator) && encoder.canEncode(SEP_HORIZONTAL) && encoder.canEncode(SEP_CROSS); } private Charset getEncoding(PrintStream ps) { - if (ps.getClass() == ThreadPrintStream.class) { + if (ps instanceof ThreadPrintStream) { try { return ((Terminal) Job.Utils.current().session().get(".jline.terminal")).encoding(); } catch (Throwable t) { // ignore } try { - ps = (PrintStream) ps.getClass().getMethod("getCurrent").invoke(ps); + ps = ((ThreadPrintStream) ps).getCurrent(); } catch (Throwable t) { // ignore } } + // FIXME: The below code produces a warning in Java 11 and might produce an error later + /* + WARNING: An illegal reflective access operation has occurred + WARNING: Illegal reflective access by org.apache.karaf.shell.support.table.ShellTable (file:/home/user/gitKaraf/shell/core/target/classes/) to field java.io.PrintStream.charOut + WARNING: Please consider reporting this to the maintainers of org.apache.karaf.shell.support.table.ShellTable + WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations + WARNING: All illegal access operations will be denied in a future release + */ try { Field f = ps.getClass().getDeclaredField("charOut"); f.setAccessible(true); @@ -211,16 +219,14 @@ public class ShellTable { return; } } - } - private String underline(int length, boolean crossAtBeg, boolean supported) { + private String underline(int length, boolean crossAtBeg, boolean unicode) { char[] exmarks = new char[length]; - Arrays.fill(exmarks, supported ? SEP_HORIZONTAL : SEP_HORIZONTAL_ASCII); + Arrays.fill(exmarks, unicode ? SEP_HORIZONTAL : SEP_HORIZONTAL_ASCII); if (crossAtBeg) { - exmarks[1] = supported ? SEP_CROSS : SEP_CROSS_ASCII; + exmarks[1] = unicode ? SEP_CROSS : SEP_CROSS_ASCII; } return new String(exmarks); } - } diff --git a/shell/core/src/main/java/org/apache/karaf/shell/support/table/StringUtil.java b/shell/core/src/main/java/org/apache/karaf/shell/support/table/StringUtil.java index b1f3ab5af9..66d2f0b27c 100644 --- a/shell/core/src/main/java/org/apache/karaf/shell/support/table/StringUtil.java +++ b/shell/core/src/main/java/org/apache/karaf/shell/support/table/StringUtil.java @@ -15,7 +15,10 @@ */ package org.apache.karaf.shell.support.table; -class StringUtil { +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +public class StringUtil { /** * Returns length of the string. @@ -35,14 +38,7 @@ class StringUtil { * @return Repeat string. */ public static String repeat(String string, int times) { - if (times <= 0) { - return ""; - } - else if (times % 2 == 0) { - return repeat(string+string, times/2); - } - else { - return string + repeat(string+string, times/2); - } + // TODO: With Java 11 this should be replaced by String.repeat() + return IntStream.range(0, times).mapToObj(i -> string).collect(Collectors.joining()); } } -- 2.19.1