Re: [PATCH v7 07/13] completion: implement and use --list-cmds=main,others

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Thu, May 10, 2018 at 10:46 AM, Nguyễn Thái Ngọc Duy
<pclouds@gmail.com> wrote:
> Instead of parsing "git help -a" output, which is tricky to get right,
> less elegant and also slow,

Is it tricky?  It seems rather straightforward.
Is it slow?  Well, there is a pipe and an egrep, sure, but thanks to
caching it's only run once, so basically doesn't matter.

OTOH you don't mention the most important reason, namely that the
completion script contains a long hard-coded list of the names of all
known plumbing commands to filter out, which is redundant with the
categorization in 'command-list.txt', is a maintenance burden, and
tends to get out-of-sync when new plumbing commands are added.

> make git provide the list in a
> machine-friendly form. This adds two separate listing types, main and
> others, instead of just "all" for more flexibility.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  contrib/completion/git-completion.bash |  2 +-
>  git.c                                  |  4 ++++
>  help.c                                 | 32 ++++++++++++++++++++++++++
>  help.h                                 |  4 ++++
>  4 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 3556838759..62ca8641f4 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -839,7 +839,7 @@ __git_commands () {
>         then
>                 printf "%s" "${GIT_TESTING_COMMAND_COMPLETION}"
>         else
> -               git help -a|egrep '^  [a-zA-Z0-9]'
> +               git --list-cmds=main,others
>         fi
>  }
>
> diff --git a/git.c b/git.c
> index 376a59b97f..10907f7266 100644
> --- a/git.c
> +++ b/git.c
> @@ -56,6 +56,10 @@ static int list_cmds(const char *spec)
>
>                 if (match_token(spec, len, "builtins"))
>                         list_builtins(&list, 0);
> +               else if (match_token(spec, len, "main"))
> +                       list_all_main_cmds(&list);
> +               else if (match_token(spec, len, "others"))
> +                       list_all_other_cmds(&list);
>                 else
>                         die(_("unsupported command listing type '%s'"), spec);
>                 spec += len;
> diff --git a/help.c b/help.c
> index 2d6a3157f8..d5ce9dfcbb 100644
> --- a/help.c
> +++ b/help.c
> @@ -297,6 +297,38 @@ void list_common_cmds_help(void)
>         print_cmd_by_category(common_categories);
>  }
>
> +void list_all_main_cmds(struct string_list *list)
> +{
> +       struct cmdnames main_cmds, other_cmds;
> +       int i;
> +
> +       memset(&main_cmds, 0, sizeof(main_cmds));
> +       memset(&other_cmds, 0, sizeof(other_cmds));
> +       load_command_list("git-", &main_cmds, &other_cmds);
> +
> +       for (i = 0; i < main_cmds.cnt; i++)
> +               string_list_append(list, main_cmds.names[i]->name);
> +
> +       clean_cmdnames(&main_cmds);
> +       clean_cmdnames(&other_cmds);
> +}
> +
> +void list_all_other_cmds(struct string_list *list)
> +{
> +       struct cmdnames main_cmds, other_cmds;
> +       int i;
> +
> +       memset(&main_cmds, 0, sizeof(main_cmds));
> +       memset(&other_cmds, 0, sizeof(other_cmds));
> +       load_command_list("git-", &main_cmds, &other_cmds);
> +
> +       for (i = 0; i < other_cmds.cnt; i++)
> +               string_list_append(list, other_cmds.names[i]->name);
> +
> +       clean_cmdnames(&main_cmds);
> +       clean_cmdnames(&other_cmds);
> +}
> +
>  int is_in_cmdlist(struct cmdnames *c, const char *s)
>  {
>         int i;
> diff --git a/help.h b/help.h
> index b21d7c94e8..97e6c0965e 100644
> --- a/help.h
> +++ b/help.h
> @@ -1,6 +1,8 @@
>  #ifndef HELP_H
>  #define HELP_H
>
> +struct string_list;
> +
>  struct cmdnames {
>         int alloc;
>         int cnt;
> @@ -17,6 +19,8 @@ static inline void mput_char(char c, unsigned int num)
>  }
>
>  extern void list_common_cmds_help(void);
> +extern void list_all_main_cmds(struct string_list *list);
> +extern void list_all_other_cmds(struct string_list *list);
>  extern const char *help_unknown_cmd(const char *cmd);
>  extern void load_command_list(const char *prefix,
>                               struct cmdnames *main_cmds,
> --
> 2.17.0.705.g3525833791
>