Re: [PATCH 4/9] help: add --config to list all available config

[Eric Sunshine <sunshine@sunshineco.com>]

On Thu, May 10, 2018 at 10:20 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
wrote:
> Sometimes it helps to list all available config vars so the user can
> search for something they want. The config man page can also be used
> but it's harder to search if you want to focus on the variable name,
> for example.

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/builtin/help.c b/builtin/help.c
> @@ -44,6 +45,7 @@ static struct option builtin_help_options[] = {
>          OPT_BOOL('g', "guides", &show_guides, N_("print list of useful
guides")),
> +       OPT_BOOL('c', "config", &show_config, N_("print list recognized
config variables")),

s/list/& of/

Though, simpler might be better: "print all configuration variable names"

> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
> @@ -76,6 +76,24 @@ print_command_list () {
> +print_config_list() {
> +       cat <<EOF
> +static const char *config_name_list[] = {
> +EOF
> +       grep '^[a-zA-Z].*\..*::$' Documentation/config.txt |
> +       grep -v deprecated |
> +       sed 's/::$//; s/,  */\n/g' |

Nit: "grep -v" and "sed" could be combined:

     sed '/deprecated/d; s/::$//; s/,  */\n/g' |

> +       sort |
> +       while read line
> +       do
> +               echo "  \"$line\","
> +       done
> +       cat <<EOF
> +       NULL,
> +};
> +EOF
> diff --git a/help.c b/help.c
> @@ -409,6 +409,54 @@ void list_common_guides_help(void)
> +void list_config_help(void)
> +{
> +       [...]
> +       for (p = config_name_list; *p; p++) {
> +               const char *var = *p;
> +
> +               for (e = slot_expansions; e->prefix; e++) {
> +                       struct strbuf sb = STRBUF_INIT;
> +
> +                       strbuf_addf(&sb, "%s.%s", e->prefix,
e->placeholder);
> +                       if (strcasecmp(var, sb.buf))
> +                               continue;

Isn't this "continue" leaking the strbuf? It seems that it would be easier
to declare the strbuf once outside the loop, strbuf_reset() it at the top
of the loop, and finally strbuf_release() it after the loop exits.

> +                       e->fn(e->prefix);
> +                       strbuf_release(&sb);
> +                       e->found++;
> +                       break;
> +               }
> +               if (!e->prefix)
> +                       puts(var);
> +       }