[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 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

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

[Burakov, Anatoly]

On 5/14/2025 3:26 PM, Bruce Richardson wrote:
> 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?
> 

Most instances of the same are already lowercase, so if we want to TM to 
be upper case, it's not just these instances that will need to be fixed. 
Also, help messages are pretty consistently lower case for all types of 
commands, so I'm not sure if making TM stand out would be better. I have 
no preference over which it is, it's just that ordering is case 
sensitive (at least when using strcmp - do we have a case-insensitive 
compare?) so some things may be ordered in a counter-intuitive way.

> /Bruce
> 


-- 
Thanks,
Anatoly

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

[Stephen Hemminger]

On Wed, 14 May 2025 14:02:12 +0100
Anatoly Burakov <anatoly.burakov@intel.com> 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>
> ---

Sorting is good, but why not pre-sort builtin_ctx and make it const?

[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

[Burakov, Anatoly]

On 5/22/2025 12:24 AM, Stephen Hemminger wrote:
> On Wed, 14 May 2025 14:02:12 +0100
> Anatoly Burakov <anatoly.burakov@intel.com> 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>
>> ---
> 
> Sorting is good, but why not pre-sort builtin_ctx and make it const?
> 

It's difficult and error-prone to sort these messages statically because 
there's no obvious to know where to insert the new value without reading 
lots of help strings from other commands. You could argue that we could 
sort by variable name but it feels like an unnecessary burden on the 
developer to maintain this sorting order when we can just automate it at 
runtime without giving up anything.

-- 
Thanks,
Anatoly

[PATCH v2 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>
---

Notes:
    v1 -> v2:
    - Make TM uniformly uppercase instead of lowercase

 app/test-pmd/cmdline.c    |  4 ++--
 app/test-pmd/cmdline_tm.c | 46 +++++++++++++++++++--------------------
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index b6152c07e6..712d307ae0 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -8112,7 +8112,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,
@@ -9068,7 +9068,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..2b165b62c6 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,
@@ -1130,7 +1130,7 @@ static void cmd_add_port_tm_node_shared_shaper_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_add_port_tm_node_shared_shaper = {
 	.f = cmd_add_port_tm_node_shared_shaper_parsed,
 	.data = NULL,
-	.help_str = "add/update port tm node shared shaper",
+	.help_str = "add/update port TM node shared shaper",
 	.tokens = {
 		(void *)&cmd_add_port_tm_node_shared_shaper_cmd_type,
 		(void *)&cmd_add_port_tm_node_shared_shaper_port,
@@ -1210,7 +1210,7 @@ static void cmd_del_port_tm_node_shared_shaper_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_del_port_tm_node_shared_shaper = {
 	.f = cmd_del_port_tm_node_shared_shaper_parsed,
 	.data = NULL,
-	.help_str = "delete port tm node shared shaper",
+	.help_str = "delete port TM node shared shaper",
 	.tokens = {
 		(void *)&cmd_del_port_tm_node_shared_shaper_del,
 		(void *)&cmd_del_port_tm_node_shared_shaper_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,
@@ -2685,7 +2685,7 @@ static void cmd_port_tm_mark_ip_ecn_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_port_tm_mark_ip_ecn = {
 	.f = cmd_port_tm_mark_ip_ecn_parsed,
 	.data = NULL,
-	.help_str = "set port tm mark ip_ecn <port> <green> <yellow> <red>",
+	.help_str = "set port TM mark ip_ecn <port> <green> <yellow> <red>",
 	.tokens = {
 		(void *)&cmd_port_tm_mark_ip_ecn_set,
 		(void *)&cmd_port_tm_mark_ip_ecn_port,
@@ -2772,7 +2772,7 @@ static void cmd_port_tm_mark_ip_dscp_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_port_tm_mark_ip_dscp = {
 	.f = cmd_port_tm_mark_ip_dscp_parsed,
 	.data = NULL,
-	.help_str = "set port tm mark ip_dscp <port> <green> <yellow> <red>",
+	.help_str = "set port TM mark ip_dscp <port> <green> <yellow> <red>",
 	.tokens = {
 		(void *)&cmd_port_tm_mark_ip_dscp_set,
 		(void *)&cmd_port_tm_mark_ip_dscp_port,
@@ -2859,7 +2859,7 @@ static void cmd_port_tm_mark_vlan_dei_parsed(void *parsed_result,
 cmdline_parse_inst_t cmd_port_tm_mark_vlan_dei = {
 	.f = cmd_port_tm_mark_vlan_dei_parsed,
 	.data = NULL,
-	.help_str = "set port tm mark vlan_dei <port> <green> <yellow> <red>",
+	.help_str = "set port TM mark vlan_dei <port> <green> <yellow> <red>",
 	.tokens = {
 		(void *)&cmd_port_tm_mark_vlan_dei_set,
 		(void *)&cmd_port_tm_mark_vlan_dei_port,
-- 
2.47.1

[PATCH v2 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>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---

Notes:
    v1 -> v2:
    - Removed outdated comment

 app/test-pmd/cmdline.c | 80 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 712d307ae0..3ed0e23080 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -13986,6 +13986,76 @@ 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_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;
+
+	ret = strcmp(l_helpstr, r_helpstr);
+	if (ret == 0)
+		return 0;
+	return ret < 0 ? -1 : 1;
+}
+
 int
 init_cmdline(void)
 {
@@ -13997,6 +14067,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++;
@@ -14010,12 +14084,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