wuchong commented on a change in pull request #16060:
URL: https://github.com/apache/flink/pull/16060#discussion_r667608901
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliStrings.java
##########
@@ -36,91 +43,149 @@ private CliStrings() {
//
--------------------------------------------------------------------------------------------
- public static final AttributedString MESSAGE_HELP =
- new AttributedStringBuilder()
- .append("The following commands are available:\n\n")
- .append(formatCommand("CLEAR", "Clears the current
terminal."))
- .append(
- formatCommand(
- "CREATE TABLE",
- "Create table under current catalog and
database."))
- .append(
- formatCommand(
- "DROP TABLE",
- "Drop table with optional catalog and
database. Syntax: \"DROP TABLE [IF EXISTS] <name>;\""))
- .append(
- formatCommand(
- "CREATE VIEW",
- "Creates a virtual table from a SQL query.
Syntax: \"CREATE VIEW <name> AS <query>;\""))
- .append(
- formatCommand(
- "DESCRIBE",
- "Describes the schema of a table with the
given name."))
- .append(
- formatCommand(
- "DROP VIEW",
- "Deletes a previously created virtual
table. Syntax: \"DROP VIEW <name>;\""))
- .append(
- formatCommand(
- "EXPLAIN",
- "Describes the execution plan of a query
or table with the given name."))
- .append(formatCommand("HELP", "Prints the available
commands."))
- .append(
- formatCommand(
+ public static final String CMD_DESC_DELIMITER = "\t\t";
+
+ /** SQL Client HELP command helper class. */
+ public static class SQLCliCommandsDescriptions {
+ private int commandMaxLength = -1;
+ private List<Tuple2<String, String>> commandsDescriptions;
+
+ public SQLCliCommandsDescriptions() {
+ commandsDescriptions = new ArrayList<>();
+ }
+
+ public SQLCliCommandsDescriptions commandDescription(
+ Tuple2<String, String> commandDescription) {
Review comment:
Separate `Tuple2` into meaningful parameters will be easier to
understand, e.g. `String cmd, String description`. Underlying, you can use
`Map` to store cmd descriptions.
##########
File path:
flink-table/flink-sql-client/src/test/java/org/apache/flink/table/client/cli/CliStringsITCase.java
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.client.cli;
+
+import org.apache.flink.shaded.guava18.com.google.common.collect.ImmutableSet;
+import org.apache.flink.shaded.guava18.com.google.common.reflect.ClassPath;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
+
+/** CliStrings test class. */
+public class CliStringsITCase {
+
+ private final String space = " ";
+ private final String empty = "";
+ final String lineBreaker = "\n";
+
+ /**
+ * Test {@link org.apache.flink.table.client.cli.CliStrings#MESSAGE_HELP}
contains all commands'
+ * help descriptions in package {@link
org.apache.flink.table.operations.command}.
+ *
+ * @throws IOException exception.
+ */
+ @Test
+ public void testCompletenessOfCliClientCommandsInHelp() throws IOException
{
+ final String operationSuffix = "OPERATION";
+ final String targetPackage =
"org.apache.flink.table.operations.command";
+ List<String> hintsInHelpMessage = getCommandsHintsInHelpMessage();
+ List<String> hintsInPackage = getCommandsHintsInPackage(targetPackage,
operationSuffix);
+ if (CollectionUtils.isNotEmpty(hintsInPackage)) {
+ if (CollectionUtils.isNotEmpty(hintsInHelpMessage)) {
+ for (String hintInPkg : hintsInPackage) {
+ boolean containable = false;
+ for (String hintInHelpMsg : hintsInHelpMessage) {
+ if (hintInHelpMsg.contains(hintInPkg)) {
+ containable = true;
+ break;
Review comment:
It seems that the check will fail if ALL hint messages don't match class
names in package. Is that on purpose?
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliStrings.java
##########
@@ -36,91 +43,149 @@ private CliStrings() {
//
--------------------------------------------------------------------------------------------
- public static final AttributedString MESSAGE_HELP =
- new AttributedStringBuilder()
- .append("The following commands are available:\n\n")
- .append(formatCommand("CLEAR", "Clears the current
terminal."))
- .append(
- formatCommand(
- "CREATE TABLE",
- "Create table under current catalog and
database."))
- .append(
- formatCommand(
- "DROP TABLE",
- "Drop table with optional catalog and
database. Syntax: \"DROP TABLE [IF EXISTS] <name>;\""))
- .append(
- formatCommand(
- "CREATE VIEW",
- "Creates a virtual table from a SQL query.
Syntax: \"CREATE VIEW <name> AS <query>;\""))
- .append(
- formatCommand(
- "DESCRIBE",
- "Describes the schema of a table with the
given name."))
- .append(
- formatCommand(
- "DROP VIEW",
- "Deletes a previously created virtual
table. Syntax: \"DROP VIEW <name>;\""))
- .append(
- formatCommand(
- "EXPLAIN",
- "Describes the execution plan of a query
or table with the given name."))
- .append(formatCommand("HELP", "Prints the available
commands."))
- .append(
- formatCommand(
+ public static final String CMD_DESC_DELIMITER = "\t\t";
+
+ /** SQL Client HELP command helper class. */
+ public static class SQLCliCommandsDescriptions {
+ private int commandMaxLength = -1;
+ private List<Tuple2<String, String>> commandsDescriptions;
Review comment:
can be `final`.
##########
File path:
flink-table/flink-sql-client/src/test/java/org/apache/flink/table/client/cli/CliStringsITCase.java
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.client.cli;
+
+import org.apache.flink.shaded.guava18.com.google.common.collect.ImmutableSet;
+import org.apache.flink.shaded.guava18.com.google.common.reflect.ClassPath;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
+
+/** CliStrings test class. */
+public class CliStringsITCase {
+
+ private final String space = " ";
+ private final String empty = "";
+ final String lineBreaker = "\n";
+
+ /**
+ * Test {@link org.apache.flink.table.client.cli.CliStrings#MESSAGE_HELP}
contains all commands'
+ * help descriptions in package {@link
org.apache.flink.table.operations.command}.
+ *
+ * @throws IOException exception.
+ */
+ @Test
+ public void testCompletenessOfCliClientCommandsInHelp() throws IOException
{
Review comment:
Personally, I think this test is not necessary and hard to maintain,
because class names and packages may change in the future. I think tests like
`sql-client-help-command.out` is enough.
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliStrings.java
##########
@@ -36,91 +43,149 @@ private CliStrings() {
//
--------------------------------------------------------------------------------------------
- public static final AttributedString MESSAGE_HELP =
- new AttributedStringBuilder()
- .append("The following commands are available:\n\n")
- .append(formatCommand("CLEAR", "Clears the current
terminal."))
- .append(
- formatCommand(
- "CREATE TABLE",
- "Create table under current catalog and
database."))
- .append(
- formatCommand(
- "DROP TABLE",
- "Drop table with optional catalog and
database. Syntax: \"DROP TABLE [IF EXISTS] <name>;\""))
- .append(
- formatCommand(
- "CREATE VIEW",
- "Creates a virtual table from a SQL query.
Syntax: \"CREATE VIEW <name> AS <query>;\""))
- .append(
- formatCommand(
- "DESCRIBE",
- "Describes the schema of a table with the
given name."))
- .append(
- formatCommand(
- "DROP VIEW",
- "Deletes a previously created virtual
table. Syntax: \"DROP VIEW <name>;\""))
- .append(
- formatCommand(
- "EXPLAIN",
- "Describes the execution plan of a query
or table with the given name."))
- .append(formatCommand("HELP", "Prints the available
commands."))
- .append(
- formatCommand(
+ public static final String CMD_DESC_DELIMITER = "\t\t";
+
+ /** SQL Client HELP command helper class. */
+ public static class SQLCliCommandsDescriptions {
+ private int commandMaxLength = -1;
+ private List<Tuple2<String, String>> commandsDescriptions;
+
+ public SQLCliCommandsDescriptions() {
+ commandsDescriptions = new ArrayList<>();
+ }
+
+ public SQLCliCommandsDescriptions commandDescription(
+ Tuple2<String, String> commandDescription) {
+ Preconditions.checkState(
+ checkCommandDescription(commandDescription),
+ "All contents of command description must not be empty.");
+ this.updateMaxCommandLength(commandDescription.f0.length());
+ this.commandsDescriptions.add(commandDescription);
+ return this;
+ }
+
+ private void updateMaxCommandLength(int newLength) {
+ Preconditions.checkState(newLength > 0);
+ if (this.commandMaxLength < newLength) {
+ this.commandMaxLength = newLength;
+ }
+ }
+
+ private boolean checkCommandDescription(Tuple2<String, String>
commandDescription) {
+ return Objects.nonNull(commandDescription)
+ && StringUtils.isNotBlank(commandDescription.f0)
+ && StringUtils.isNotBlank(commandDescription.f1);
+ }
+
+ public void clearCommandsDescriptions() {
+ this.commandsDescriptions.clear();
+ this.resetMaxCommandLength();
+ }
+
+ private void resetMaxCommandLength() {
+ this.commandMaxLength = -1;
+ }
+
+ public int getCommandMaxLength() {
+ return this.commandMaxLength;
+ }
+
+ public AttributedString formatCommand(
+ String cmdDescDelimiter,
+ String cmdFormat,
+ String descFormat,
+ boolean clearCmdsDescs) {
+ Preconditions.checkNotNull(cmdDescDelimiter, "delimiter must not
be null.");
+ Preconditions.checkState(
+ StringUtils.isNotBlank(cmdFormat), "command format must
not be null.");
+ Preconditions.checkState(
+ StringUtils.isNotBlank(descFormat),
+ "command description format must not be null.");
+ AttributedStringBuilder attributedStringBuilder = new
AttributedStringBuilder();
+ if (!this.commandsDescriptions.isEmpty()) {
+ this.commandsDescriptions.forEach(
+ cmdDescTuple2 -> {
+ attributedStringBuilder
+ .style(AttributedStyle.DEFAULT.bold())
+ .append(String.format(cmdFormat,
cmdDescTuple2.f0))
+ .append(CMD_DESC_DELIMITER)
+ .style(AttributedStyle.DEFAULT)
+ .append(String.format(descFormat,
cmdDescTuple2.f1))
+ .append('\n');
+ });
+ }
+ if (clearCmdsDescs) {
+ this.clearCommandsDescriptions();
Review comment:
Why we need to clear the command descriptions? We don't allow the
`formatCommand` to be called multiple times?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
