[PATCH v1 1/2] app/testpmd: harmonize case in help strings

[PATCH v1 1/2] app/testpmd: harmonize case in help strings

[Anatoly Burakov]

Most testpmd help strings are lower case. Amend those that aren't.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 app/test-pmd/cmdline.c    |  4 ++--
 app/test-pmd/cmdline_tm.c | 36 ++++++++++++++++++------------------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index d4bb3ec998..a8f1b8ad67 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -8078,7 +8078,7 @@ static cmdline_parse_token_string_t cmd_operate_set_counter_name =
 static cmdline_parse_inst_t cmd_operate_set_counter = {
 	.f = cmd_operate_set_counter_parsed,
 	.data = NULL,
-	.help_str = "port (port_id) enable|disable <counter_name>",
+	.help_str = "port <port_id> enable|disable <counter_name>",
 	.tokens = {
 		(void *)&cmd_operate_set_counter_port,
 		(void *)&cmd_operate_set_counter_port_id,
@@ -9034,7 +9034,7 @@ static cmdline_parse_token_string_t cmd_dump_dump =
 static cmdline_parse_inst_t cmd_dump = {
 	.f = cmd_dump_parsed,  /* function to call */
 	.data = NULL,      /* 2nd arg of func */
-	.help_str = "Dump status",
+	.help_str = "dump status",
 	.tokens = {        /* token list, NULL terminated */
 		(void *)&cmd_dump_dump,
 		NULL,
diff --git a/app/test-pmd/cmdline_tm.c b/app/test-pmd/cmdline_tm.c
index 7ade91549c..acf86c1ef0 100644
--- a/app/test-pmd/cmdline_tm.c
+++ b/app/test-pmd/cmdline_tm.c
@@ -327,7 +327,7 @@ static void cmd_show_port_tm_cap_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_show_port_tm_cap = {
 	.f = cmd_show_port_tm_cap_parsed,
 	.data = NULL,
-	.help_str = "Show Port TM Capabilities",
+	.help_str = "show port tm capabilities",
 	.tokens = {
 		(void *)&cmd_show_port_tm_cap_show,
 		(void *)&cmd_show_port_tm_cap_port,
@@ -475,7 +475,7 @@ static void cmd_show_port_tm_level_cap_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_show_port_tm_level_cap = {
 	.f = cmd_show_port_tm_level_cap_parsed,
 	.data = NULL,
-	.help_str = "Show port TM hierarchical level capabilities",
+	.help_str = "show port tm hierarchical level capabilities",
 	.tokens = {
 		(void *)&cmd_show_port_tm_level_cap_show,
 		(void *)&cmd_show_port_tm_level_cap_port,
@@ -598,7 +598,7 @@ static void cmd_show_port_tm_node_cap_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_show_port_tm_node_cap = {
 	.f = cmd_show_port_tm_node_cap_parsed,
 	.data = NULL,
-	.help_str = "Show Port TM Hierarchy node capabilities",
+	.help_str = "show port tm Hierarchy node capabilities",
 	.tokens = {
 		(void *)&cmd_show_port_tm_node_cap_show,
 		(void *)&cmd_show_port_tm_node_cap_port,
@@ -716,7 +716,7 @@ static void cmd_show_port_tm_node_stats_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_show_port_tm_node_stats = {
 	.f = cmd_show_port_tm_node_stats_parsed,
 	.data = NULL,
-	.help_str = "Show port tm node stats",
+	.help_str = "show port tm node stats",
 	.tokens = {
 		(void *)&cmd_show_port_tm_node_stats_show,
 		(void *)&cmd_show_port_tm_node_stats_port,
@@ -795,7 +795,7 @@ static void cmd_show_port_tm_node_type_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_show_port_tm_node_type = {
 	.f = cmd_show_port_tm_node_type_parsed,
 	.data = NULL,
-	.help_str = "Show port tm node type",
+	.help_str = "show port tm node type",
 	.tokens = {
 		(void *)&cmd_show_port_tm_node_type_show,
 		(void *)&cmd_show_port_tm_node_type_port,
@@ -942,7 +942,7 @@ static void cmd_add_port_tm_node_shaper_profile_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_add_port_tm_node_shaper_profile = {
 	.f = cmd_add_port_tm_node_shaper_profile_parsed,
 	.data = NULL,
-	.help_str = "Add port tm node private shaper profile",
+	.help_str = "add port tm node private shaper profile",
 	.tokens = {
 		(void *)&cmd_add_port_tm_node_shaper_profile_add,
 		(void *)&cmd_add_port_tm_node_shaper_profile_port,
@@ -1029,7 +1029,7 @@ static void cmd_del_port_tm_node_shaper_profile_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_del_port_tm_node_shaper_profile = {
 	.f = cmd_del_port_tm_node_shaper_profile_parsed,
 	.data = NULL,
-	.help_str = "Delete port tm node private shaper profile",
+	.help_str = "delete port tm node private shaper profile",
 	.tokens = {
 		(void *)&cmd_del_port_tm_node_shaper_profile_del,
 		(void *)&cmd_del_port_tm_node_shaper_profile_port,
@@ -1390,7 +1390,7 @@ static void cmd_add_port_tm_node_wred_profile_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_add_port_tm_node_wred_profile = {
 	.f = cmd_add_port_tm_node_wred_profile_parsed,
 	.data = NULL,
-	.help_str = "Add port tm node wred profile",
+	.help_str = "add port tm node wred profile",
 	.tokens = {
 		(void *)&cmd_add_port_tm_node_wred_profile_add,
 		(void *)&cmd_add_port_tm_node_wred_profile_port,
@@ -1483,7 +1483,7 @@ static void cmd_del_port_tm_node_wred_profile_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_del_port_tm_node_wred_profile = {
 	.f = cmd_del_port_tm_node_wred_profile_parsed,
 	.data = NULL,
-	.help_str = "Delete port tm node wred profile",
+	.help_str = "delete port tm node wred profile",
 	.tokens = {
 		(void *)&cmd_del_port_tm_node_wred_profile_del,
 		(void *)&cmd_del_port_tm_node_wred_profile_port,
@@ -1577,7 +1577,7 @@ static void cmd_set_port_tm_node_shaper_profile_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_set_port_tm_node_shaper_profile = {
 	.f = cmd_set_port_tm_node_shaper_profile_parsed,
 	.data = NULL,
-	.help_str = "Set port tm node shaper profile",
+	.help_str = "set port tm node shaper profile",
 	.tokens = {
 		(void *)&cmd_set_port_tm_node_shaper_profile_set,
 		(void *)&cmd_set_port_tm_node_shaper_profile_port,
@@ -1744,7 +1744,7 @@ static void cmd_add_port_tm_nonleaf_node_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_add_port_tm_nonleaf_node = {
 	.f = cmd_add_port_tm_nonleaf_node_parsed,
 	.data = NULL,
-	.help_str = "Add port tm nonleaf node",
+	.help_str = "add port tm nonleaf node",
 	.tokens = {
 		(void *)&cmd_add_port_tm_nonleaf_node_add,
 		(void *)&cmd_add_port_tm_nonleaf_node_port,
@@ -1911,7 +1911,7 @@ static void cmd_add_port_tm_nonleaf_node_pmode_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_add_port_tm_nonleaf_node_pmode = {
 	.f = cmd_add_port_tm_nonleaf_node_pmode_parsed,
 	.data = NULL,
-	.help_str = "Add port tm nonleaf node pktmode",
+	.help_str = "add port tm nonleaf node pktmode",
 	.tokens = {
 		(void *)&cmd_add_port_tm_nonleaf_node_pmode_add,
 		(void *)&cmd_add_port_tm_nonleaf_node_pmode_port,
@@ -2089,7 +2089,7 @@ static void cmd_add_port_tm_leaf_node_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_add_port_tm_leaf_node = {
 	.f = cmd_add_port_tm_leaf_node_parsed,
 	.data = NULL,
-	.help_str = "Add port tm leaf node",
+	.help_str = "add port tm leaf node",
 	.tokens = {
 		(void *)&cmd_add_port_tm_leaf_node_add,
 		(void *)&cmd_add_port_tm_leaf_node_port,
@@ -2301,7 +2301,7 @@ static void cmd_del_port_tm_node_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_del_port_tm_node = {
 	.f = cmd_del_port_tm_node_parsed,
 	.data = NULL,
-	.help_str = "Delete port tm node",
+	.help_str = "delete port tm node",
 	.tokens = {
 		(void *)&cmd_del_port_tm_node_del,
 		(void *)&cmd_del_port_tm_node_port,
@@ -2394,7 +2394,7 @@ static void cmd_set_port_tm_node_parent_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_set_port_tm_node_parent = {
 	.f = cmd_set_port_tm_node_parent_parsed,
 	.data = NULL,
-	.help_str = "Set port tm node parent",
+	.help_str = "set port tm node parent",
 	.tokens = {
 		(void *)&cmd_set_port_tm_node_parent_set,
 		(void *)&cmd_set_port_tm_node_parent_port,
@@ -2465,7 +2465,7 @@ static void cmd_suspend_port_tm_node_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_suspend_port_tm_node = {
 	.f = cmd_suspend_port_tm_node_parsed,
 	.data = NULL,
-	.help_str = "Suspend port tm node",
+	.help_str = "suspend port tm node",
 	.tokens = {
 		(void *)&cmd_suspend_port_tm_node_suspend,
 		(void *)&cmd_suspend_port_tm_node_port,
@@ -2530,7 +2530,7 @@ static void cmd_resume_port_tm_node_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_resume_port_tm_node = {
 	.f = cmd_resume_port_tm_node_parsed,
 	.data = NULL,
-	.help_str = "Resume port tm node",
+	.help_str = "resume port tm node",
 	.tokens = {
 		(void *)&cmd_resume_port_tm_node_resume,
 		(void *)&cmd_resume_port_tm_node_port,
@@ -2602,7 +2602,7 @@ static void cmd_port_tm_hierarchy_commit_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_port_tm_hierarchy_commit = {
 	.f = cmd_port_tm_hierarchy_commit_parsed,
 	.data = NULL,
-	.help_str = "Commit port tm hierarchy",
+	.help_str = "commit port tm hierarchy",
 	.tokens = {
 		(void *)&cmd_port_tm_hierarchy_commit_port,
 		(void *)&cmd_port_tm_hierarchy_commit_tm,
-- 
2.47.1

[PATCH v1 2/2] app/testpmd: sort commands by help string

[Anatoly Burakov]

When using '?' to find commands, it occasionally is difficult to find the
needed commands because all commands are not in alphabetical order, but
rather can be ordered rather arbitrarily.

To address this, use help string to order commands. This sacrifices some
amount of grouping (i.e. when tm commands go one after another), but may
improve discoverability (and most similar commands tend to have similar
help strings and will be located closer together anyway).

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 app/test-pmd/cmdline.c | 82 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a8f1b8ad67..c91fe732b6 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -13951,6 +13951,78 @@ testpmd_add_driver_commands(struct testpmd_driver_commands *c)
 	TAILQ_INSERT_TAIL(&driver_commands_head, c, next);
 }
 
+static int
+cmd_cmp(const void *l, const void *r)
+{
+	cmdline_parse_inst_t *l_cmd = *(cmdline_parse_inst_t * const*)l;
+	cmdline_parse_inst_t *r_cmd = *(cmdline_parse_inst_t * const*)r;
+	int l_idx = -1, r_idx = -1;
+	size_t i;
+
+	/* these commands should always be at the top of the list */
+	cmdline_parse_inst_t const *top_cmds[] = {
+		&cmd_help_brief,
+		&cmd_help_long,
+		&cmd_quit,
+		&cmd_load_from_file,
+		&cmd_start,
+		&cmd_stop,
+	};
+
+	/* there may be NULL values - push them to the end */
+	if (l_cmd == NULL)
+		return 1;
+	if (r_cmd == NULL)
+		return -1;
+
+	/* should circuit on equality - shouldn't happen anyway */
+	if (l_cmd == r_cmd)
+		return 0;
+
+	/* try finding both commands in list header */
+	for (i = 0; i < RTE_DIM(top_cmds); i++) {
+		cmdline_parse_inst_t const *tmp = top_cmds[i];
+		if (l_idx == -1 && l_cmd == tmp)
+			l_idx = i;
+		if (r_idx == -1 && r_cmd == tmp)
+			r_idx = i;
+	}
+
+	if (l_idx >= 0 && r_idx < 0)
+		/* l in list, r not in list */
+		return -1;
+	if (l_idx < 0 && r_idx >= 0)
+		/* l not in list, r in list */
+		return 1;
+	if (l_idx >= 0 && r_idx >= 0)
+		/* both in list */
+		return l_idx < r_idx ? -1 : 1;
+
+	/*
+	 * Neither are in top list. Comparing commands is difficult because
+	 * there are multiple types of tokens, some of which are custom and
+	 * cannot be generalized. What we'll do instead is compare help strings,
+	 * because those tend to be fairly consistently specified.
+	 */
+	const char *l_helpstr = l_cmd->help_str;
+	const char *r_helpstr = r_cmd->help_str;
+	int ret;
+
+	/* some commands do not have help strings, push them to the back */
+	if (l_helpstr == NULL && r_helpstr == NULL)
+		return 0;
+	if (l_helpstr == NULL)
+		return 1;
+	if (r_helpstr == NULL)
+		return -1;
+
+	/* we need case-insensitive compare */
+	ret = strcmp(l_helpstr, r_helpstr);
+	if (ret == 0)
+		return 0;
+	return ret < 0 ? -1 : 1;
+}
+
 int
 init_cmdline(void)
 {
@@ -13962,6 +14034,10 @@ init_cmdline(void)
 	cmd_set_fwd_mode_init();
 	cmd_set_fwd_retry_mode_init();
 
+	/* sort built-in command array */
+	qsort(builtin_ctx, RTE_DIM(builtin_ctx) - 1, sizeof(builtin_ctx[0]), cmd_cmp);
+
+	/* count total number of commands */
 	count = 0;
 	for (i = 0; builtin_ctx[i] != NULL; i++)
 		count++;
@@ -13975,12 +14051,18 @@ init_cmdline(void)
 	if (main_ctx == NULL)
 		return -1;
 
+	/* copy built-in commands */
 	count = 0;
 	for (i = 0; builtin_ctx[i] != NULL; i++, count++)
 		main_ctx[count] = builtin_ctx[i];
+
+	/* copy driver-specific commands */
 	TAILQ_FOREACH(c, &driver_commands_head, next) {
+		size_t start = count;
 		for (i = 0; c->commands[i].ctx != NULL; i++, count++)
 			main_ctx[count] = c->commands[i].ctx;
+		/* sort driver-specific commands, per driver */
+		qsort(&main_ctx[start], i, sizeof(main_ctx[0]), cmd_cmp);
 	}
 
 	return 0;
-- 
2.47.1

Re: [PATCH v1 2/2] app/testpmd: sort commands by help string

[Bruce Richardson]

On Wed, May 14, 2025 at 02:02:12PM +0100, Anatoly Burakov wrote:
> When using '?' to find commands, it occasionally is difficult to find the
> needed commands because all commands are not in alphabetical order, but
> rather can be ordered rather arbitrarily.
> 
> To address this, use help string to order commands. This sacrifices some
> amount of grouping (i.e. when tm commands go one after another), but may
> improve discoverability (and most similar commands tend to have similar
> help strings and will be located closer together anyway).
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  app/test-pmd/cmdline.c | 82 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
> 

+1 to sorting
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [PATCH v1 1/2] app/testpmd: harmonize case in help strings

[Bruce Richardson]

On Wed, May 14, 2025 at 02:02:11PM +0100, Anatoly Burakov wrote:
> Most testpmd help strings are lower case. Amend those that aren't.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  app/test-pmd/cmdline.c    |  4 ++--
>  app/test-pmd/cmdline_tm.c | 36 ++++++++++++++++++------------------
>  2 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index d4bb3ec998..a8f1b8ad67 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -8078,7 +8078,7 @@ static cmdline_parse_token_string_t cmd_operate_set_counter_name =
>  static cmdline_parse_inst_t cmd_operate_set_counter = {
>  	.f = cmd_operate_set_counter_parsed,
>  	.data = NULL,
> -	.help_str = "port (port_id) enable|disable <counter_name>",
> +	.help_str = "port <port_id> enable|disable <counter_name>",
>  	.tokens = {
>  		(void *)&cmd_operate_set_counter_port,
>  		(void *)&cmd_operate_set_counter_port_id,
> @@ -9034,7 +9034,7 @@ static cmdline_parse_token_string_t cmd_dump_dump =
>  static cmdline_parse_inst_t cmd_dump = {
>  	.f = cmd_dump_parsed,  /* function to call */
>  	.data = NULL,      /* 2nd arg of func */
> -	.help_str = "Dump status",
> +	.help_str = "dump status",
>  	.tokens = {        /* token list, NULL terminated */
>  		(void *)&cmd_dump_dump,
>  		NULL,
> diff --git a/app/test-pmd/cmdline_tm.c b/app/test-pmd/cmdline_tm.c
> index 7ade91549c..acf86c1ef0 100644
> --- a/app/test-pmd/cmdline_tm.c
> +++ b/app/test-pmd/cmdline_tm.c
> @@ -327,7 +327,7 @@ static void cmd_show_port_tm_cap_parsed(void *parsed_result,
>  cmdline_parse_inst_t cmd_show_port_tm_cap = {
>  	.f = cmd_show_port_tm_cap_parsed,
>  	.data = NULL,
> -	.help_str = "Show Port TM Capabilities",
> +	.help_str = "show port tm capabilities",

Not so certain about this change. Should acronyms like "TM" not be
capitalised, to make it clear they are not normal words?

/Bruce