help: cleanup the contruction of keys_uniq

construction of keys_uniq depends on sort operation
executed on keys before processing, which does not
gurantee that keys_uniq will be sorted.

refactor the code to shift the sort operation after
the processing to remove dependency on key's sort operation
and strictly maintain the sorted order of keys_uniq.

move strbuf init and release out of loop to reuse same buffer.

dedent sort -u and sed in tests and replace grep with sed, to
avoid piping grep's output to sed.

Suggested-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
Signed-off-by: Amisha Chhajed <amishhhaaaa@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/help.c b/builtin/help.c
index c09cbc8912deb69acf25a748684c639cbec1955e..daadc61c70ae786fa31dd8defb3df9781a662f6b 100644 (file)
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -111,6 +111,49 @@ struct slot_expansion {
        int found;
 };
 
+static void set_config_vars(struct string_list *keys_uniq, struct string_list_item *var)
+{
+       struct strbuf sb = STRBUF_INIT;
+       const char *str = var->string;
+       const char *wildcard = strchr(str, '*');
+       const char *tag = strchr(str, '<');
+       const char *cut;
+
+       if (wildcard && tag)
+               cut = wildcard < tag ? wildcard : tag;
+       else if (wildcard)
+               cut = wildcard;
+       else if (tag)
+               cut = tag;
+       else {
+               string_list_append(keys_uniq, str);
+               return;
+       }
+
+       strbuf_add(&sb, str, cut - str);
+       string_list_append(keys_uniq, sb.buf);
+       strbuf_release(&sb);
+}
+
+static void set_config_sections(struct string_list *keys_uniq, struct string_list_item *var)
+{
+       struct strbuf sb = STRBUF_INIT;
+       const char *str = var->string;
+       const char *dot = strchr(str, '.');
+       const char *cut;
+
+       if (dot)
+               cut = dot;
+       else {
+               set_config_vars(keys_uniq, var);
+               return;
+       }
+
+       strbuf_add(&sb, str, cut - str);
+       string_list_append(keys_uniq, sb.buf);
+       strbuf_release(&sb);
+}
+
 static void list_config_help(enum show_config_type type)
 {
        struct slot_expansion slot_expansions[] = {
@@ -131,13 +174,12 @@ static void list_config_help(enum show_config_type type)
        struct string_list keys = STRING_LIST_INIT_DUP;
        struct string_list keys_uniq = STRING_LIST_INIT_DUP;
        struct string_list_item *item;
+       struct strbuf sb = STRBUF_INIT;
 
        for (p = config_name_list; *p; p++) {
                const char *var = *p;
-               struct strbuf sb = STRBUF_INIT;
 
                for (e = slot_expansions; e->prefix; e++) {
-
                        strbuf_reset(&sb);
                        strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
                        if (!strcasecmp(var, sb.buf)) {
@@ -146,60 +188,39 @@ static void list_config_help(enum show_config_type type)
                                break;
                        }
                }
-               strbuf_release(&sb);
+
                if (!e->prefix)
                        string_list_append(&keys, var);
        }
 
+       strbuf_release(&sb);
+
        for (e = slot_expansions; e->prefix; e++)
                if (!e->found)
                        BUG("slot_expansion %s.%s is not used",
                            e->prefix, e->placeholder);
 
-       string_list_sort(&keys);
        for (size_t i = 0; i < keys.nr; i++) {
-               const char *var = keys.items[i].string;
-               const char *wildcard, *tag, *cut;
-               const char *dot = NULL;
-               struct strbuf sb = STRBUF_INIT;
-
                switch (type) {
                case SHOW_CONFIG_HUMAN:
-                       puts(var);
-                       continue;
+                       string_list_append(&keys_uniq, keys.items[i].string);
+                       break;
                case SHOW_CONFIG_SECTIONS:
-                       dot = strchr(var, '.');
+                       set_config_sections(&keys_uniq, &keys.items[i]);
                        break;
                case SHOW_CONFIG_VARS:
+                       set_config_vars(&keys_uniq, &keys.items[i]);
                        break;
+               default:
+                       BUG("%d: unexpected type", type);
                }
-               wildcard = strchr(var, '*');
-               tag = strchr(var, '<');
-
-               if (!dot && !wildcard && !tag) {
-                       string_list_append(&keys_uniq, var);
-                       continue;
-               }
-
-               if (dot)
-                       cut = dot;
-               else if (wildcard && !tag)
-                       cut = wildcard;
-               else if (!wildcard && tag)
-                       cut = tag;
-               else
-                       cut = wildcard < tag ? wildcard : tag;
-
-               strbuf_add(&sb, var, cut - var);
-               string_list_append(&keys_uniq, sb.buf);
-               strbuf_release(&sb);
-
        }
-       string_list_clear(&keys, 0);
-       string_list_remove_duplicates(&keys_uniq, 0);
+
+       string_list_sort_u(&keys_uniq, 0);
        for_each_string_list_item(item, &keys_uniq)
                puts(item->string);
        string_list_clear(&keys_uniq, 0);
+       string_list_clear(&keys, 0);
 }
 
 static enum help_format parse_help_format(const char *format)

diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index d3a0967e9d075254d0a2fbecb6d30b00e0e2010f..c33501bdcd2b9b64cb945aae17ed8fac753b2d63 100755 (executable)
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -141,20 +141,23 @@ test_expect_success 'git help -c' '
 
        '\''git help config'\'' for more information
        EOF
-       grep -v -E \
-               -e "^[^.]+\.[^.]+$" \
-               -e "^[^.]+\.[^.]+\.[^.]+$" \
-               help.output >actual &&
+       sed -E -e "
+               /^[^.]+\.[^.]+$/d
+               /^[^.]+\.[^.]+\.[^.]+$/d
+       " help.output >actual &&
        test_cmp expect actual
 '
 
 test_expect_success 'git help --config-for-completion' '
        git help -c >human &&
-       grep -E \
-            -e "^[^.]+\.[^.]+$" \
-            -e "^[^.]+\.[^.]+\.[^.]+$" human |
-            sed -e "s/\*.*//" -e "s/<.*//" |
-            sort -u >human.munged &&
+       sed -E -e "
+               /^[^.]+\.[^.]+$/b out
+               /^[^.]+\.[^.]+\.[^.]+$/b out
+               d
+               : out
+               s/\*.*//
+               s/<.*//
+       " human | sort -u >human.munged &&
 
        git help --config-for-completion >vars &&
        test_cmp human.munged vars
@@ -162,14 +165,16 @@ test_expect_success 'git help --config-for-completion' '
 
 test_expect_success 'git help --config-sections-for-completion' '
        git help -c >human &&
-       grep -E \
-            -e "^[^.]+\.[^.]+$" \
-            -e "^[^.]+\.[^.]+\.[^.]+$" human |
-            sed -e "s/\..*//" |
-            sort -u >human.munged &&
-
-       git help --config-sections-for-completion >sections &&
-       test_cmp human.munged sections
+       sed -E -e "
+               /^[^.]+\.[^.]+$/b out
+               /^[^.]+\.[^.]+\.[^.]+$/b out
+               d
+               : out
+               s/\..*//
+       " human | sort -u >expect &&
+
+       git help --config-sections-for-completion >actual &&
+       test_cmp expect actual
 '
 
 test_section_spacing () {