Re: [PATCH v3 2/4] generate-cmdlist.sh: parse common command groups

[Junio C Hamano <gitster@pobox.com>]

Sébastien Guimmara  <sebastien.guimmara@gmail.com> writes:

> Teach generate-cmdlist.sh to parse common command groups
> found in command-list.txt in the form
>
> common-3_worktree ('3_worktree' being the group identifier)
>
> Extract the $grp variable, in addition to the previous $cmd,
> and inject it as a third field in the cmdname_help struct:
>
> {"add", N_("Add file contents to the index"), "3_worktree"},
>
> So that when 'git' is called, we can display common commands
> grouped by theme instead of a less useful alphabetical order.
>
> Reviewed by: Luke Diamand <luke@diamand.org>
> Reviewed by: Andreas Schwab <schwab@linux-m68k.org>

These people may have helped you to polish your earlier round to
come up with this version, but I do not think they should be listed
as reviewed-by (in the sense that they would say "Yes, I read this
version and consider it very good--I endorse the change!") yet.

> Signed-off-by: Sébastien Guimmara <sebastien.guimmara@gmail.com>
> ---

>  generate-cmdlist.sh | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
> index 9a4c9b9..98f937b 100755
> --- a/generate-cmdlist.sh
> +++ b/generate-cmdlist.sh
> @@ -4,19 +4,20 @@ echo "/* Automatically generated by $0 */
>  struct cmdname_help {
>      char name[16];
>      char help[80];
> +    char group[20];

Storing group name in duplicated text for all commands and then
using string comparison at runtime is grossly wasteful, don't you
think, especially when the list is not manually maintained?

command-list.txt is something manually maintained, i.e. the source,
and it makes sense to use easy-to-see strings like "3_worktree"
label there instead of cryptic number, but the whole point of this
script is to pre-process that text into .c file to a form that is
more suited for machine consumption.  I think this script should:

 - read command list once to make the list of groups in a new
   separate array;

 - add the group as an "unsigned char group_number" (as we won't
   have more than 200 groups) field to this struct;

 - read command list again and for each command give the group
   number here.

or something like that.