{"thread":{"id":"65214","subject":"[PATCH v5] help: cleanup the contruction of keys_uniq","startedAt":"2026-03-11T19:22:22Z","lastAt":"2026-03-11T19:22:22Z","messageCount":1,"participants":["Amisha Chhajed"],"isPatch":true,"patchVersion":5,"patchTotal":null},"messages":[{"id":"538661","messageId":"20260311192151.60489-1-amishhhaaaa@gmail.com","threadId":"65214","inReplyTo":null,"subject":"[PATCH v5] help: cleanup the contruction of keys_uniq","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-03-11T19:21:51Z","receivedAt":"2026-03-11T19:22:22Z","isPatch":true,"sender":{"key":"amishhhaaaa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/136238836?v=4"},"body":"From: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n\nconstruction of keys_uniq depends on sort operation\nexecuted on keys before processing, which does not\ngurantee that keys_uniq will be sorted.\n\nrefactor the code to shift the sort operation after\nthe processing to remove dependency on key's sort operation\nand strictly maintain the sorted order of keys_uniq.\n\nmove strbuf init and release out of loop to reuse same buffer.\n\ndedent sort -u and sed in tests and replace grep with sed, to\navoid piping grep's output to sed.\n\nSuggested-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>\nSigned-off-by: Amisha Chhajed <amishhhaaaa@gmail.com>\n---\n builtin/help.c  | 91 ++++++++++++++++++++++++++++++-------------------\n t/t0012-help.sh | 39 ++++++++++++---------\n 2 files changed, 78 insertions(+), 52 deletions(-)\n\ndiff --git a/builtin/help.c b/builtin/help.c\nindex c09cbc8912..daadc61c70 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -111,6 +111,49 @@ struct slot_expansion {\n \tint found;\n };\n \n+static void set_config_vars(struct string_list *keys_uniq, struct string_list_item *var)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *str = var->string;\n+\tconst char *wildcard = strchr(str, '*');\n+\tconst char *tag = strchr(str, '<');\n+\tconst char *cut;\n+\n+\tif (wildcard && tag)\n+\t\tcut = wildcard < tag ? wildcard : tag;\n+\telse if (wildcard)\n+\t\tcut = wildcard;\n+\telse if (tag)\n+\t\tcut = tag;\n+\telse {\n+\t\tstring_list_append(keys_uniq, str);\n+\t\treturn;\n+\t}\n+\n+\tstrbuf_add(&sb, str, cut - str);\n+\tstring_list_append(keys_uniq, sb.buf);\n+\tstrbuf_release(&sb);\n+}\n+\n+static void set_config_sections(struct string_list *keys_uniq, struct string_list_item *var)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *str = var->string;\n+\tconst char *dot = strchr(str, '.');\n+\tconst char *cut;\n+\n+\tif (dot)\n+\t\tcut = dot;\n+\telse {\n+\t\tset_config_vars(keys_uniq, var);\n+\t\treturn;\n+\t}\n+\n+\tstrbuf_add(&sb, str, cut - str);\n+\tstring_list_append(keys_uniq, sb.buf);\n+\tstrbuf_release(&sb);\n+}\n+\n static void list_config_help(enum show_config_type type)\n {\n \tstruct slot_expansion slot_expansions[] = {\n@@ -131,13 +174,12 @@ static void list_config_help(enum show_config_type type)\n \tstruct string_list keys = STRING_LIST_INIT_DUP;\n \tstruct string_list keys_uniq = STRING_LIST_INIT_DUP;\n \tstruct string_list_item *item;\n+\tstruct strbuf sb = STRBUF_INIT;\n \n \tfor (p = config_name_list; *p; p++) {\n \t\tconst char *var = *p;\n-\t\tstruct strbuf sb = STRBUF_INIT;\n \n \t\tfor (e = slot_expansions; e->prefix; e++) {\n-\n \t\t\tstrbuf_reset(&sb);\n \t\t\tstrbuf_addf(&sb, \"%s.%s\", e->prefix, e->placeholder);\n \t\t\tif (!strcasecmp(var, sb.buf)) {\n@@ -146,60 +188,39 @@ static void list_config_help(enum show_config_type type)\n \t\t\t\tbreak;\n \t\t\t}\n \t\t}\n-\t\tstrbuf_release(&sb);\n+\n \t\tif (!e->prefix)\n \t\t\tstring_list_append(&keys, var);\n \t}\n \n+\tstrbuf_release(&sb);\n+\n \tfor (e = slot_expansions; e->prefix; e++)\n \t\tif (!e->found)\n \t\t\tBUG(\"slot_expansion %s.%s is not used\",\n \t\t\t    e->prefix, e->placeholder);\n \n-\tstring_list_sort(&keys);\n \tfor (size_t i = 0; i < keys.nr; i++) {\n-\t\tconst char *var = keys.items[i].string;\n-\t\tconst char *wildcard, *tag, *cut;\n-\t\tconst char *dot = NULL;\n-\t\tstruct strbuf sb = STRBUF_INIT;\n-\n \t\tswitch (type) {\n \t\tcase SHOW_CONFIG_HUMAN:\n-\t\t\tputs(var);\n-\t\t\tcontinue;\n+\t\t\tstring_list_append(&keys_uniq, keys.items[i].string);\n+\t\t\tbreak;\n \t\tcase SHOW_CONFIG_SECTIONS:\n-\t\t\tdot = strchr(var, '.');\n+\t\t\tset_config_sections(&keys_uniq, &keys.items[i]);\n \t\t\tbreak;\n \t\tcase SHOW_CONFIG_VARS:\n+\t\t\tset_config_vars(&keys_uniq, &keys.items[i]);\n \t\t\tbreak;\n+\t\tdefault:\n+\t\t\tBUG(\"%d: unexpected type\", type);\n \t\t}\n-\t\twildcard = strchr(var, '*');\n-\t\ttag = strchr(var, '<');\n-\n-\t\tif (!dot && !wildcard && !tag) {\n-\t\t\tstring_list_append(&keys_uniq, var);\n-\t\t\tcontinue;\n-\t\t}\n-\n-\t\tif (dot)\n-\t\t\tcut = dot;\n-\t\telse if (wildcard && !tag)\n-\t\t\tcut = wildcard;\n-\t\telse if (!wildcard && tag)\n-\t\t\tcut = tag;\n-\t\telse\n-\t\t\tcut = wildcard < tag ? wildcard : tag;\n-\n-\t\tstrbuf_add(&sb, var, cut - var);\n-\t\tstring_list_append(&keys_uniq, sb.buf);\n-\t\tstrbuf_release(&sb);\n-\n \t}\n-\tstring_list_clear(&keys, 0);\n-\tstring_list_remove_duplicates(&keys_uniq, 0);\n+\n+\tstring_list_sort_u(&keys_uniq, 0);\n \tfor_each_string_list_item(item, &keys_uniq)\n \t\tputs(item->string);\n \tstring_list_clear(&keys_uniq, 0);\n+\tstring_list_clear(&keys, 0);\n }\n \n static enum help_format parse_help_format(const char *format)\ndiff --git a/t/t0012-help.sh b/t/t0012-help.sh\nindex d3a0967e9d..40b2d656a5 100755\n--- a/t/t0012-help.sh\n+++ b/t/t0012-help.sh\n@@ -141,20 +141,23 @@ test_expect_success 'git help -c' '\n \n \t'\\''git help config'\\'' for more information\n \tEOF\n-\tgrep -v -E \\\n-\t\t-e \"^[^.]+\\.[^.]+$\" \\\n-\t\t-e \"^[^.]+\\.[^.]+\\.[^.]+$\" \\\n-\t\thelp.output >actual &&\n+\tsed -E -e \"\n+\t\t/^[^.]+\\.[^.]+$/d\n+\t\t/^[^.]+\\.[^.]+\\.[^.]+$/d\n+\t\" help.output >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'git help --config-for-completion' '\n \tgit help -c >human &&\n-\tgrep -E \\\n-\t     -e \"^[^.]+\\.[^.]+$\" \\\n-\t     -e \"^[^.]+\\.[^.]+\\.[^.]+$\" human |\n-\t     sed -e \"s/\\*.*//\" -e \"s/<.*//\" |\n-\t     sort -u >human.munged &&\n+\tsed -E -e \"\n+\t\t/^[^.]+\\.[^.]+$/b out\n+\t\t/^[^.]+\\.[^.]+\\.[^.]+$/b out\n+\t\td\n+\t\t: out\n+\t\ts/\\*.*//\n+\t\ts/<.*//\n+\t\" human | sort -u >human.munged &&\n \n \tgit help --config-for-completion >vars &&\n \ttest_cmp human.munged vars\n@@ -162,14 +165,16 @@ test_expect_success 'git help --config-for-completion' '\n \n test_expect_success 'git help --config-sections-for-completion' '\n \tgit help -c >human &&\n-\tgrep -E \\\n-\t     -e \"^[^.]+\\.[^.]+$\" \\\n-\t     -e \"^[^.]+\\.[^.]+\\.[^.]+$\" human |\n-\t     sed -e \"s/\\..*//\" |\n-\t     sort -u >human.munged &&\n-\n-\tgit help --config-sections-for-completion >sections &&\n-\ttest_cmp human.munged sections\n+\tsed -E -e \"\n+\t\t/^[^.]+\\.[^.]+$/b out\n+\t\t/^[^.]+\\.[^.]+\\.[^.]+$/b out\n+\t\td\n+\t\t: out\n+\t\ts/\\..*//\n+\t\" human | sort -u >expect &&\n+\t\n+\tgit help --config-sections-for-completion >actual &&\n+\ttest_cmp expect actual\n '\n \n test_section_spacing () {\n-- \n2.52.0\n\n"}]}