{"thread":{"id":"64983","subject":"[PATCH 0/2] clean leftover calls to string_list_remove_duplicates","startedAt":"2026-02-12T04:10:34Z","lastAt":"2026-03-11T21:55:05Z","messageCount":31,"participants":["Amisha Chhajed","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"535817","messageId":"20260212041017.91370-1-amishhhaaaa@gmail.com","threadId":"64983","inReplyTo":null,"subject":"[PATCH 0/2] clean leftover calls to string_list_remove_duplicates","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-02-12T04:10:15Z","receivedAt":"2026-02-12T04:10:34Z","isPatch":true,"sender":{"key":"amishhhaaaa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/136238836?v=4"},"body":"replace calls to string_list_remove_duplicates with string_list_sort_u.\n\nsorted behavior of &keys_uniq depends on the call to string_list_sort on\n&keys before &keys is processed to form &keys_uniq, this introduces an\nedge case where &keys_uniq will not be sorted as expected, follow [1]\nto reproduce.\nadd string_list_sort_u to &keys_uniq to fix this edge case and ensure\nfuture enhancements to the processing logic does not introduce regressions.\n\n[1]\nrun command:\ncat <<'EOF' >> Documentation/config/add.adoc\naa*.b::\naa.b::\nEOF\nfrom git/ and then make test, this sees one failure when the test compared\nactual output with sort -u output.\n\nAmisha Chhajed (2):\n  sparse-checkout: use string_list_sort_u\n  help: ensure &keys_uniq follows sort -u\n\n builtin/help.c            | 2 +-\n builtin/sparse-checkout.c | 3 +--\n 2 files changed, 2 insertions(+), 3 deletions(-)\n\n-- \n2.52.0\n\n"},{"id":"535818","messageId":"20260212041017.91370-2-amishhhaaaa@gmail.com","threadId":"64983","inReplyTo":"20260212041017.91370-1-amishhhaaaa@gmail.com","subject":"[PATCH 1/2] sparse-checkout: use string_list_sort_u","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-02-12T04:10:16Z","receivedAt":"2026-02-12T04:10:39Z","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\nsparse_checkout_list() uses string_list_sort and\nstring_list_remove_duplicates instead of string_list_sort_u.\n\nuse string_list_sort_u at that place.\n\nSigned-off-by: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n---\n builtin/sparse-checkout.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex cccf630331..34e965bfa6 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -94,8 +94,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,\n \t\t\tstring_list_append(&sl, pe->pattern + 1);\n \t\t}\n \n-\t\tstring_list_sort(&sl);\n-\t\tstring_list_remove_duplicates(&sl, 0);\n+\t\tstring_list_sort_u(&sl, 0);\n \n \t\tfor (i = 0; i < sl.nr; i++) {\n \t\t\tquote_c_style(sl.items[i].string, NULL, stdout, 0);\n-- \n2.52.0\n\n"},{"id":"535819","messageId":"20260212041017.91370-3-amishhhaaaa@gmail.com","threadId":"64983","inReplyTo":"20260212041017.91370-1-amishhhaaaa@gmail.com","subject":"[PATCH 2/2] help: ensure &keys_uniq follows sort -u","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-02-12T04:10:17Z","receivedAt":"2026-02-12T04:10:43Z","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\nuniqueness operation of &keys_uniq depends on the sort operation executed\nfor &keys this might introduce regressions in future when the logic of\nforming &keys_uniq from &keys is changed.\n\nadd string_list_sort_u operation for &keys_uniq after the processing of\n&keys so it follows the expected sort -u behaviour.\n\nSigned-off-by: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n---\n builtin/help.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/help.c b/builtin/help.c\nindex c09cbc8912..0c9c007214 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -196,7 +196,7 @@ static void list_config_help(enum show_config_type type)\n \n \t}\n \tstring_list_clear(&keys, 0);\n-\tstring_list_remove_duplicates(&keys_uniq, 0);\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-- \n2.52.0\n\n"},{"id":"535881","messageId":"xmqqo6ltoj3m.fsf@gitster.g","threadId":"64983","inReplyTo":"20260212041017.91370-2-amishhhaaaa@gmail.com","subject":"Re: [PATCH 1/2] sparse-checkout: use string_list_sort_u","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-12T19:30:21Z","receivedAt":"2026-02-12T19:30:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n\n> From: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n>\n> sparse_checkout_list() uses string_list_sort and\n> string_list_remove_duplicates instead of string_list_sort_u.\n>\n> use string_list_sort_u at that place.\n>\n> Signed-off-by: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n> ---\n>  builtin/sparse-checkout.c | 3 +--\n>  1 file changed, 1 insertion(+), 2 deletions(-)\n>\n> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\n> index cccf630331..34e965bfa6 100644\n> --- a/builtin/sparse-checkout.c\n> +++ b/builtin/sparse-checkout.c\n> @@ -94,8 +94,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,\n>  \t\t\tstring_list_append(&sl, pe->pattern + 1);\n>  \t\t}\n>  \n> -\t\tstring_list_sort(&sl);\n> -\t\tstring_list_remove_duplicates(&sl, 0);\n> +\t\tstring_list_sort_u(&sl, 0);\n>  \n>  \t\tfor (i = 0; i < sl.nr; i++) {\n>  \t\t\tquote_c_style(sl.items[i].string, NULL, stdout, 0);\n\nObviously correct.  Will queue.  Thanks.\n"},{"id":"535882","messageId":"xmqqh5rlohsm.fsf@gitster.g","threadId":"64983","inReplyTo":"20260212041017.91370-3-amishhhaaaa@gmail.com","subject":"Re: [PATCH 2/2] help: ensure &keys_uniq follows sort -u","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-12T19:58:33Z","receivedAt":"2026-02-12T19:58:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n\n> From: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n>\n> uniqueness operation of &keys_uniq depends on the sort operation executed\n> for &keys this might introduce regressions in future when the logic of\n> forming &keys_uniq from &keys is changed.\n>\n> add string_list_sort_u operation for &keys_uniq after the processing of\n> &keys so it follows the expected sort -u behaviour.\n\nI am not sure the above reasoning is sound.  With the original code,\nwe\n\n - prepare empty keys_uniq\n - collect keys\n - sort keys\n - iterate over keys\n   - add either the whole \"section[.subsection].key\" or \"section\" to keys_uniq\n\nbefore we call remove_duplicates.  keys_uniq would have duplicates,\nbut because keys is sorted upfront, wouldn't the contents of\nkeys_uniq be collected in sorted order anyway?\n\nThis is not a performance critical part of the system, so it is OK\nas a future-proof measure to sort keys_uniq immediately before we\nstart doing something that we _care_ about its sortedness (e.g.,\npresenting the final output to the user), even if keys_uniq is known\nto be already sorted with the current code.  Using sort_u here would\nallow us not to worry about how keys_uniq is constructed in that\nugly loop.\n\nYes, this function, especially the loop before the part you are\ntouching, _is_ ugly.  What drug the authors of it were under when it\nwas written, I have to wonder X-<.  For example, wouldn't readers\nwonder why CONFIG_HUMAN output mode does puts() right in the middle\nof the loop over keys string list, while the other two does not\nputs() and have a separate loop over keys_uniq instead?\n\nI suspect that making a switch(type) that calls one of three helper\nfunctions for the three different output types after keys has been\npopulated in the earlier part of this function, but immediately\nbefore it is sorted with string_list_sort(&keys), would be a\nlow-hanging fruit clean-up that makes the result far easier to\nfollow than the current code.  The helper function to handle\nCONFIG_HUMAN mode may need to sort keys, but other two helper\nfunctions do not have to and iterate over unsorted keys to construct\ntheir output list, on which they can do sort_u before they output.\n\nThanks.\n\n> Signed-off-by: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n> ---\n>  builtin/help.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/help.c b/builtin/help.c\n> index c09cbc8912..0c9c007214 100644\n> --- a/builtin/help.c\n> +++ b/builtin/help.c\n> @@ -196,7 +196,7 @@ static void list_config_help(enum show_config_type type)\n>  \n>  \t}\n>  \tstring_list_clear(&keys, 0);\n> -\tstring_list_remove_duplicates(&keys_uniq, 0);\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"},{"id":"535888","messageId":"CAPvEtrenMBMFaMxcCR4VwoyMFU-_Z+bqq5nJaWv5eyn3HRutEA@mail.gmail.com","threadId":"64983","inReplyTo":"xmqqh5rlohsm.fsf@gitster.g","subject":"Re: [PATCH 2/2] help: ensure &keys_uniq follows sort -u","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-02-12T21:29:43Z","receivedAt":"2026-02-12T21:29:55Z","isPatch":true,"sender":{"key":"amishhhaaaa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/136238836?v=4"},"body":"On Fri, 13 Feb 2026 at 01:28, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n>\n> > From: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n> >\n> > uniqueness operation of &keys_uniq depends on the sort operation executed\n> > for &keys this might introduce regressions in future when the logic of\n> > forming &keys_uniq from &keys is changed.\n> >\n> > add string_list_sort_u operation for &keys_uniq after the processing of\n> > &keys so it follows the expected sort -u behaviour.\n>\n> I am not sure the above reasoning is sound.  With the original code,\n> we\n>\n>  - prepare empty keys_uniq\n>  - collect keys\n>  - sort keys\n>  - iterate over keys\n>    - add either the whole \"section[.subsection].key\" or \"section\" to keys_uniq\n>\n> before we call remove_duplicates.  keys_uniq would have duplicates,\n> but because keys is sorted upfront, wouldn't the contents of\n> keys_uniq be collected in sorted order anyway?\n\nNo, there is a case where it would not be sorted(keys_uniq won't be sorted\neven though keys is), more details on the case[0] and steps to reproduce[1].\n[0] https://lore.kernel.org/git/CAPvEtrfEZXHxcDf=z60ODfUA8cS81rhF1y7KEZApEBby7aCa1A@mail.gmail.com/\n[1] https://lore.kernel.org/git/20260212041017.91370-1-amishhhaaaa@gmail.com/T/#m64880c5cd0d36e35bc78692757cf206b13496aea\nonly reason it is not causing a problem now is because we do not have\nthis edge case appearing git documentation(from where the keys are built)\nbut if someday a case like this appears there then it would cause problems.\n\n> This is not a performance critical part of the system, so it is OK\n> as a future-proof measure to sort keys_uniq immediately before we\n> start doing something that we _care_ about its sortedness (e.g.,\n> presenting the final output to the user), even if keys_uniq is known\n> to be already sorted with the current code.  Using sort_u here would\n> allow us not to worry about how keys_uniq is constructed in that\n> ugly loop.\n\nAgreed, we do not need to sort it twice if we decouple CONFIG_HUMAN\nfrom the rest of the switch case, that is a great way to go about it,\nthank you!.\nI will work on it.\n"},{"id":"535890","messageId":"xmqqcy29myo0.fsf@gitster.g","threadId":"64983","inReplyTo":"CAPvEtrenMBMFaMxcCR4VwoyMFU-_Z+bqq5nJaWv5eyn3HRutEA@mail.gmail.com","subject":"Re: [PATCH 2/2] help: ensure &keys_uniq follows sort -u","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-12T21:37:03Z","receivedAt":"2026-02-12T21:37:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n\n> No, there is a case where it would not be sorted(keys_uniq won't be sorted\n> even though keys is), more details on the case[0] and steps to reproduce[1].\n> [0] https://lore.kernel.org/git/CAPvEtrfEZXHxcDf=z60ODfUA8cS81rhF1y7KEZApEBby7aCa1A@mail.gmail.com/\n> [1] https://lore.kernel.org/git/20260212041017.91370-1-amishhhaaaa@gmail.com/T/#m64880c5cd0d36e35bc78692757cf206b13496aea\n> only reason it is not causing a problem now is because we do not have\n> this edge case appearing git documentation(from where the keys are built)\n> but if someday a case like this appears there then it would cause problems.\n\nAh, if you already have a reproduction case , it would have been\nvery good to add it as a new test.  That way, we can (1) apply the\npatch, (2) tentatively revert only the code change, (3) build and\nrun test to see that the test breaks, demonstrating an existing\nbreakage, (4) restore the code change we tentatively reverted, (5)\nbuild and run test again to see that the existing breakage is now\ngone.\n\n>> This is not a performance critical part of the system, so it is OK\n>> as a future-proof measure to sort keys_uniq immediately before we\n>> start doing something that we _care_ about its sortedness (e.g.,\n>> presenting the final output to the user), even if keys_uniq is known\n>> to be already sorted with the current code.  Using sort_u here would\n>> allow us not to worry about how keys_uniq is constructed in that\n>> ugly loop.\n>\n> Agreed, we do not need to sort it twice if we decouple CONFIG_HUMAN\n> from the rest of the switch case, that is a great way to go about it,\n> thank you!.\n> I will work on it.\n\nThanks.\n"},{"id":"535903","messageId":"20260213033729.50208-1-amishhhaaaa@gmail.com","threadId":"64983","inReplyTo":"20260212041017.91370-1-amishhhaaaa@gmail.com","subject":"[PATCH v2 1/2] sparse-checkout: use string_list_sort_u","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-02-13T03:37:28Z","receivedAt":"2026-02-13T03:37:45Z","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\nsparse_checkout_list() uses string_list_sort and\nstring_list_remove_duplicates instead of string_list_sort_u.\n\nuse string_list_sort_u at that place.\n\nSigned-off-by: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n---\n builtin/sparse-checkout.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex cccf630331..34e965bfa6 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -94,8 +94,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,\n \t\t\tstring_list_append(&sl, pe->pattern + 1);\n \t\t}\n \n-\t\tstring_list_sort(&sl);\n-\t\tstring_list_remove_duplicates(&sl, 0);\n+\t\tstring_list_sort_u(&sl, 0);\n \n \t\tfor (i = 0; i < sl.nr; i++) {\n \t\t\tquote_c_style(sl.items[i].string, NULL, stdout, 0);\n-- \n2.52.0\n\n"},{"id":"535904","messageId":"20260213033729.50208-2-amishhhaaaa@gmail.com","threadId":"64983","inReplyTo":"20260213033729.50208-1-amishhhaaaa@gmail.com","subject":"[PATCH v2 2/2] help: cleanup the contruction of keys_uniq","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-02-13T03:37:29Z","receivedAt":"2026-02-13T03:37:54Z","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\nuniqueness property of keys_uniq depends on the sort operation executed\nfor keys, sorted property of keys does not gurantee sorted property of\nkeys_uniq due to processing keys, this might also introduce regressions\nin future when the logic of forming keys_uniq from keys is changed.\n\nadd string_list_sort_u operation for keys_uniq and refactor the\nprocessing code to simplify it, add test that demonstrates that sorted\nproperty of keys does not gurantee sorted property of keys_uniq.\n\nSigned-off-by: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n---\n builtin/help.c  | 71 +++++++++++++++++++++++++------------------------\n t/t0012-help.sh | 18 +++++++++++++\n 2 files changed, 54 insertions(+), 35 deletions(-)\n\ndiff --git a/builtin/help.c b/builtin/help.c\nindex c09cbc8912..c278d7ffcb 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -156,47 +156,48 @@ static void list_config_help(enum show_config_type type)\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+\tif (type == SHOW_CONFIG_HUMAN) {\n+\t\tstring_list_sort(&keys);\n+\t\tfor (size_t i = 0; i < keys.nr; i++) {\n+\t\t\tconst char *var = keys.items[i].string;\n \t\t\tputs(var);\n-\t\t\tcontinue;\n-\t\tcase SHOW_CONFIG_SECTIONS:\n-\t\t\tdot = strchr(var, '.');\n-\t\t\tbreak;\n-\t\tcase SHOW_CONFIG_VARS:\n-\t\t\tbreak;\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+\t}\n+\telse{\n+\t\tfor (size_t i = 0; i < keys.nr; i++) {\n+\t\t\tconst char *var = keys.items[i].string;\n+\t\t\tconst char *wildcard, *tag, *cut;\n+\t\t\tconst char *dot = NULL;\n+\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\t\tif (type == SHOW_CONFIG_SECTIONS) {\n+\t\t\t\tdot = strchr(var, '.');\n+\t\t\t}\n+\t\t\twildcard = strchr(var, '*');\n+\t\t\ttag = strchr(var, '<');\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+\t\t\tif (!dot && !wildcard && !tag) {\n+\t\t\t\tstring_list_append(&keys_uniq, var);\n+\t\t\t\tcontinue;\n+\t\t\t}\n \n+\t\t\tif (dot)\n+\t\t\t\tcut = dot;\n+\t\t\telse if (wildcard && !tag)\n+\t\t\t\tcut = wildcard;\n+\t\t\telse if (!wildcard && tag)\n+\t\t\t\tcut = tag;\n+\t\t\telse\n+\t\t\t\tcut = wildcard < tag ? wildcard : tag;\n+\n+\t\t\tstrbuf_add(&sb, var, cut - var);\n+\t\t\tstring_list_append(&keys_uniq, sb.buf);\n+\t\t\tstrbuf_release(&sb);\n+\t\t}\n \t}\n+\n \tstring_list_clear(&keys, 0);\n-\tstring_list_remove_duplicates(&keys_uniq, 0);\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);\ndiff --git a/t/t0012-help.sh b/t/t0012-help.sh\nindex d3a0967e9d..0dbe6dd46f 100755\n--- a/t/t0012-help.sh\n+++ b/t/t0012-help.sh\n@@ -160,6 +160,24 @@ test_expect_success 'git help --config-for-completion' '\n \ttest_cmp human.munged vars\n '\n \n+test_expect_success 'git help --config-for-completion' '\n+\tfile=\"$GIT_SOURCE_DIR/Documentation/config/add.adoc\" &&\n+\ttest_when_finished \"git -C \\\"$GIT_SOURCE_DIR\\\" checkout -- Documentation/config/add.adoc\" &&\n+\tcat <<-\\EOF >>\"$file\" &&\n+\taa*.b::\n+\taa.b::\n+\tEOF\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+\n+\tgit help --config-for-completion >vars &&\n+\ttest_cmp human.munged vars\n+'\n+\n test_expect_success 'git help --config-sections-for-completion' '\n \tgit help -c >human &&\n \tgrep -E \\\n-- \n2.52.0\n\n"},{"id":"535905","messageId":"xmqqecmpnu3g.fsf@gitster.g","threadId":"64983","inReplyTo":"20260213033729.50208-2-amishhhaaaa@gmail.com","subject":"Re: [PATCH v2 2/2] help: cleanup the contruction of keys_uniq","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-13T04:30:27Z","receivedAt":"2026-02-13T04:30:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n\n> +\t}\n> +\telse{\n\nStyle:\n\n\t} else {\n\n> +\t\tfor (size_t i = 0; i < keys.nr; i++) {\n> +\t\t\tconst char *var = keys.items[i].string;\n> +\t\t\tconst char *wildcard, *tag, *cut;\n> +\t\t\tconst char *dot = NULL;\n> +\t\t\tstruct strbuf sb = STRBUF_INIT;\n> +\n> +\t\t\tif (type == SHOW_CONFIG_SECTIONS) {\n> +\t\t\t\tdot = strchr(var, '.');\n> +\t\t\t}\n\nNo {braces} around a single-statement block.  Wouldn't it easier to\nfollow if we do not rely on the initialization?  I.e.,\n\n\t\t\tif (type == SHOW_CONFIG_SECTIONS)\n\t\t\t\tdot = strchr(var, '.');\n\t\t\telse /* SHOW_CONFIG_VARS */\n\t\t\t\tdot = NULL;\n\nor even\n\n   \t\t\tswitch (type) {\n                        case SHOW_CONFIG_SECTIONS:\n\t\t\t\tdot = strchr(var, '.');\n\t\t\t\tbreak;\n\t\t\tcase SHOW_CONFIG_VARS:\n\t\t\t\tdot = NULL;\n\t\t\t\tbreak;\n\t\t\tdefault:\n\t\t\t\tBUG(\"%d: unexpected type\", type);\n\t\t\t}\n\n?\n\nBut all of the above might become a moot point; see below.\n\n> +\t\t\twildcard = strchr(var, '*');\n> +\t\t\ttag = strchr(var, '<');\n>  \n> +\t\t\tif (!dot && !wildcard && !tag) {\n> +\t\t\t\tstring_list_append(&keys_uniq, var);\n> +\t\t\t\tcontinue;\n> +\t\t\t}\n>  \n> +\t\t\tif (dot)\n> +\t\t\t\tcut = dot;\n> +\t\t\telse if (wildcard && !tag)\n> +\t\t\t\tcut = wildcard;\n> +\t\t\telse if (!wildcard && tag)\n> +\t\t\t\tcut = tag;\n> +\t\t\telse\n> +\t\t\t\tcut = wildcard < tag ? wildcard : tag;\n\nHow much are you saving by conflating SHOW_CONFIG_SECTIONS and\nSHOW_CONFIG_VARS into this same \"else\" block?  In CONFIG_VARS mode,\ndot is always NULL, and when dot is not NULL, neither wildcard or\ntag affect the final output at all.  Would the logic become clearer\nif you split these two mode into two, I have to wonder?\n\n> +\t\t\tstrbuf_add(&sb, var, cut - var);\n> +\t\t\tstring_list_append(&keys_uniq, sb.buf);\n> +\t\t\tstrbuf_release(&sb);\n> +\t\t}\n>  \t}\n> +\n>  \tstring_list_clear(&keys, 0);\n> -\tstring_list_remove_duplicates(&keys_uniq, 0);\n> +\tstring_list_sort_u(&keys_uniq, 0);\n>  \tfor_each_string_list_item(item, &keys_uniq)\n>  \t\tputs(item->string);\n\nYou inherited the source of ugliness from the original.  Even in\nHUMAN mode, you sort-u keys_uniq and run puts(), and the only thing\nthat makes it a no-op is the fact that in the if/else above, keys_uniq\nis left untouched.\n\nI wonder if the above should look more like this:\n\n\tswitch (type) {\n\tcase SHOW_CONFIG_HUMAN:\n\t\tshow_config_human(&keys);\n\t\tbreak;\n\tcase SHOW_CONFIG_SECTIONS:\n\t\tshow_config_sections(&keys);\n\t\tbreak;\n\tcase SHOW_CONFIG_VARS:\n\t\tshow_config_vars(&keys);\n\t\tbreak;\n\tdefault:\n\t\tBUG(\"%d: unexpected type\", type);\n\t}\n\tstring_list_clear(&keys);\n        return;\n\nwithout keys_uniq string list in this function (it would be an\nimplementation detail in show_config_sections() and _vars().\n\nq> diff --git a/t/t0012-help.sh b/t/t0012-help.sh\n> index d3a0967e9d..0dbe6dd46f 100755\n> --- a/t/t0012-help.sh\n> +++ b/t/t0012-help.sh\n> @@ -160,6 +160,24 @@ test_expect_success 'git help --config-for-completion' '\n>  \ttest_cmp human.munged vars\n>  '\n>  \n> +test_expect_success 'git help --config-for-completion' '\n> +\tfile=\"$GIT_SOURCE_DIR/Documentation/config/add.adoc\" &&\n> +\ttest_when_finished \"git -C \\\"$GIT_SOURCE_DIR\\\" checkout -- Documentation/config/add.adoc\" &&\n> +\tcat <<-\\EOF >>\"$file\" &&\n> +\taa*.b::\n> +\taa.b::\n> +\tEOF\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\nDedent \"sed\" and \"sort\" to the same level as \"grep -E\".\n\n> +\tgit help --config-for-completion >vars &&\n> +\ttest_cmp human.munged vars\n> +'\n> +\n>  test_expect_success 'git help --config-sections-for-completion' '\n>  \tgit help -c >human &&\n>  \tgrep -E \\\n"},{"id":"535906","messageId":"CAPig+cRciH+qvjXTcW-32b2-QtK41rYXZosjNXy2mC0AijajKQ@mail.gmail.com","threadId":"64983","inReplyTo":"xmqqecmpnu3g.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] help: cleanup the contruction of keys_uniq","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2026-02-13T05:02:25Z","receivedAt":"2026-02-13T05:02:37Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Feb 12, 2026 at 11:30 PM Junio C Hamano <gitster@pobox.com> wrote:\n> Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n> > +test_expect_success 'git help --config-for-completion' '\n> > +     file=\"$GIT_SOURCE_DIR/Documentation/config/add.adoc\" &&\n> > +     test_when_finished \"git -C \\\"$GIT_SOURCE_DIR\\\" checkout -- Documentation/config/add.adoc\" &&\n> > +     cat <<-\\EOF >>\"$file\" &&\n> > +     aa*.b::\n> > +     aa.b::\n> > +     EOF\n> > +     git help -c >human &&\n> > +     grep -E \\\n> > +          -e \"^[^.]+\\.[^.]+$\" \\\n> > +          -e \"^[^.]+\\.[^.]+\\.[^.]+$\" human |\n> > +          sed -e \"s/\\*.*//\" -e \"s/<.*//\" |\n> > +          sort -u >human.munged &&\n>\n> Dedent \"sed\" and \"sort\" to the same level as \"grep -E\".\n\nAlso, don't we usually avoid having both `grep` and `sed` in the same\npipeline like this, considering that `sed` alone should be able to\nhandle the job itself?\n"},{"id":"535942","messageId":"xmqqa4xcoa3e.fsf@gitster.g","threadId":"64983","inReplyTo":"CAPig+cRciH+qvjXTcW-32b2-QtK41rYXZosjNXy2mC0AijajKQ@mail.gmail.com","subject":"Re: [PATCH v2 2/2] help: cleanup the contruction of keys_uniq","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-13T16:57:09Z","receivedAt":"2026-02-13T16:57:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Thu, Feb 12, 2026 at 11:30 PM Junio C Hamano <gitster@pobox.com> wrote:\n>> Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n>> > +test_expect_success 'git help --config-for-completion' '\n>> > +     file=\"$GIT_SOURCE_DIR/Documentation/config/add.adoc\" &&\n>> > +     test_when_finished \"git -C \\\"$GIT_SOURCE_DIR\\\" checkout -- Documentation/config/add.adoc\" &&\n>> > +     cat <<-\\EOF >>\"$file\" &&\n>> > +     aa*.b::\n>> > +     aa.b::\n>> > +     EOF\n>> > +     git help -c >human &&\n>> > +     grep -E \\\n>> > +          -e \"^[^.]+\\.[^.]+$\" \\\n>> > +          -e \"^[^.]+\\.[^.]+\\.[^.]+$\" human |\n>> > +          sed -e \"s/\\*.*//\" -e \"s/<.*//\" |\n>> > +          sort -u >human.munged &&\n>>\n>> Dedent \"sed\" and \"sort\" to the same level as \"grep -E\".\n>\n> Also, don't we usually avoid having both `grep` and `sed` in the same\n> pipeline like this, considering that `sed` alone should be able to\n> handle the job itself?\n\nYes, we often say \"do not pipe output of grep or awk to sed\".  I did\nnot want to burden a bit too much on a contributor who is relatively\nnew to the list.\n\nThanks.\n"},{"id":"536591","messageId":"20260221162359.43336-1-amishhhaaaa@gmail.com","threadId":"64983","inReplyTo":"20260212041017.91370-1-amishhhaaaa@gmail.com","subject":"[PATCH v3 1/2] sparse-checkout: use string_list_sort_u","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-02-21T16:23:58Z","receivedAt":"2026-02-21T16:24:25Z","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\nsparse_checkout_list() uses string_list_sort and\nstring_list_remove_duplicates instead of string_list_sort_u.\n\nuse string_list_sort_u at that place.\n\nSigned-off-by: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n---\n builtin/sparse-checkout.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex cccf630331..34e965bfa6 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -94,8 +94,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,\n \t\t\tstring_list_append(&sl, pe->pattern + 1);\n \t\t}\n \n-\t\tstring_list_sort(&sl);\n-\t\tstring_list_remove_duplicates(&sl, 0);\n+\t\tstring_list_sort_u(&sl, 0);\n \n \t\tfor (i = 0; i < sl.nr; i++) {\n \t\t\tquote_c_style(sl.items[i].string, NULL, stdout, 0);\n-- \n2.52.0\n\n"},{"id":"536592","messageId":"20260221162359.43336-2-amishhhaaaa@gmail.com","threadId":"64983","inReplyTo":"20260221162359.43336-1-amishhhaaaa@gmail.com","subject":"[PATCH v3 2/2] help: cleanup the contruction of keys_uniq","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-02-21T16:23:59Z","receivedAt":"2026-02-21T16:24:36Z","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\nuniqueness property of keys_uniq depends on the sort operation executed\nfor keys, sorted property of keys does not gurantee sorted property of\nkeys_uniq due to processing keys, this might also introduce regressions\nin future when the logic of forming keys_uniq from keys is changed.\n\nadd string_list_sort_u operation for keys_uniq and refactor the\nprocessing code to simplify it.\n\nSigned-off-by: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n---\n builtin/help.c | 134 +++++++++++++++++++++++++++++++++----------------\n 1 file changed, 90 insertions(+), 44 deletions(-)\n\ndiff --git a/builtin/help.c b/builtin/help.c\nindex c09cbc8912..b70de09864 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -111,6 +111,84 @@ struct slot_expansion {\n \tint found;\n };\n \n+static void show_config_human(struct string_list *keys)\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\tputs(var);\n+\t}\n+}\n+\n+static void show_config_sections(struct string_list *keys)\n+{\n+\tstruct string_list keys_uniq = STRING_LIST_INIT_DUP;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct string_list_item *item;\n+\n+\tfor (size_t i = 0; i < keys->nr; i++) {\n+\t\tconst char *var = keys->items[i].string;\n+\t\tconst char *dot = strchr(var, '.');\n+\t\tconst char *wildcard = strchr(var, '*');\n+\t\tconst char *tag = strchr(var, '<');\n+\t\tconst char *cut;\n+\n+\t\tif (dot)\n+\t\t\tcut = dot;\n+\t\telse if (wildcard && tag)\n+\t\t\tcut = wildcard < tag ? wildcard : tag;\n+\t\telse if (wildcard)\n+\t\t\tcut = wildcard;\n+\t\telse if (tag)\n+\t\t\tcut = tag;\n+\t\telse {\n+\t\t\tstring_list_append(&keys_uniq, var);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tstrbuf_add(&sb, var, cut - var);\n+\t\tstring_list_append(&keys_uniq, sb.buf);\n+\t\tstrbuf_release(&sb);\n+\t}\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+}\n+\n+static void show_config_vars(struct string_list *keys)\n+{\n+\tstruct string_list keys_uniq = STRING_LIST_INIT_DUP;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct string_list_item *item;\n+\n+\tfor (size_t i = 0; i < keys->nr; i++) {\n+\t\tconst char *var = keys->items[i].string;\n+\t\tconst char *wildcard = strchr(var, '*');\n+\t\tconst char *tag = strchr(var, '<');\n+\t\tconst char *cut;\n+\n+\t\tif (wildcard && tag)\n+\t\t\tcut = wildcard < tag ? wildcard : tag;\n+\t\telse if (wildcard)\n+\t\t\tcut = wildcard;\n+\t\telse if (tag)\n+\t\t\tcut = tag;\n+\t\telse {\n+\t\t\tstring_list_append(&keys_uniq, var);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tstrbuf_add(&sb, var, cut - var);\n+\t\tstring_list_append(&keys_uniq, sb.buf);\n+\t\tstrbuf_release(&sb);\n+\t}\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+}\n+\n static void list_config_help(enum show_config_type type)\n {\n \tstruct slot_expansion slot_expansions[] = {\n@@ -129,8 +207,6 @@ static void list_config_help(enum show_config_type type)\n \tconst char **p;\n \tstruct slot_expansion *e;\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 \n \tfor (p = config_name_list; *p; p++) {\n \t\tconst char *var = *p;\n@@ -156,50 +232,20 @@ static void list_config_help(enum show_config_type type)\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\tcase SHOW_CONFIG_SECTIONS:\n-\t\t\tdot = strchr(var, '.');\n-\t\t\tbreak;\n-\t\tcase SHOW_CONFIG_VARS:\n-\t\t\tbreak;\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+\tswitch (type) {\n+\tcase SHOW_CONFIG_HUMAN:\n+\t\tshow_config_human(&keys);\n+\t\tbreak;\n+\tcase SHOW_CONFIG_SECTIONS:\n+\t\tshow_config_sections(&keys);\n+\t\tbreak;\n+\tcase SHOW_CONFIG_VARS:\n+\t\tshow_config_vars(&keys);\n+\t\tbreak;\n+\tdefault:\n+\t\tBUG(\"%d: unexpected type\", type);\n \t}\n \tstring_list_clear(&keys, 0);\n-\tstring_list_remove_duplicates(&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 }\n \n static enum help_format parse_help_format(const char *format)\n-- \n2.52.0\n\n"},{"id":"536593","messageId":"CAPvEtrf37yJ2T2EsM3sgDodO=kdu_C5eXT9dmvcepzZwhAMzWQ@mail.gmail.com","threadId":"64983","inReplyTo":"xmqqecmpnu3g.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] help: cleanup the contruction of keys_uniq","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-02-21T16:28:28Z","receivedAt":"2026-02-21T16:28:40Z","isPatch":true,"sender":{"key":"amishhhaaaa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/136238836?v=4"},"body":">\n> q> diff --git a/t/t0012-help.sh b/t/t0012-help.sh\n> > index d3a0967e9d..0dbe6dd46f 100755\n> > --- a/t/t0012-help.sh\n> > +++ b/t/t0012-help.sh\n> > @@ -160,6 +160,24 @@ test_expect_success 'git help --config-for-completion' '\n> >       test_cmp human.munged vars\n> >  '\n> >\n> > +test_expect_success 'git help --config-for-completion' '\n> > +     file=\"$GIT_SOURCE_DIR/Documentation/config/add.adoc\" &&\n> > +     test_when_finished \"git -C \\\"$GIT_SOURCE_DIR\\\" checkout -- Documentation/config/add.adoc\" &&\n> > +     cat <<-\\EOF >>\"$file\" &&\n> > +     aa*.b::\n> > +     aa.b::\n> > +     EOF\n> > +     git help -c >human &&\n> > +     grep -E \\\n> > +          -e \"^[^.]+\\.[^.]+$\" \\\n> > +          -e \"^[^.]+\\.[^.]+\\.[^.]+$\" human |\n> > +          sed -e \"s/\\*.*//\" -e \"s/<.*//\" |\n> > +          sort -u >human.munged &&\n>\n> Dedent \"sed\" and \"sort\" to the same level as \"grep -E\".\n>\n> > +     git help --config-for-completion >vars &&\n> > +     test_cmp human.munged vars\n> > +'\n> > +\n> >  test_expect_success 'git help --config-sections-for-completion' '\n> >       git help -c >human &&\n> >       grep -E \\\n\n\nhad to drop this test, as it was working for a while locally on my\nmachine because i had manually\nadded the case in documentation then didn't make clean so the binary\nhad it. Unfortunately i wasn't\nable to find a way that rebuilds config-list.h to test this, noticed\nthat this topic is actively being worked on\nin https://lore.kernel.org/git/9cdcc9de04f0f8fff657f0474b31c063466ed808.1771280837.git.ben.knoble+github@gmail.com/T/#me826da3b6a128e1ceb7215d64328b7d6aa2b211e\n"},{"id":"536626","messageId":"xmqq8qclsdjd.fsf@gitster.g","threadId":"64983","inReplyTo":"20260221162359.43336-1-amishhhaaaa@gmail.com","subject":"Re: [PATCH v3 1/2] sparse-checkout: use string_list_sort_u","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-22T02:44:06Z","receivedAt":"2026-02-22T02:44:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n\n> From: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n>\n> sparse_checkout_list() uses string_list_sort and\n> string_list_remove_duplicates instead of string_list_sort_u.\n>\n> use string_list_sort_u at that place.\n>\n> Signed-off-by: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n> ---\n>  builtin/sparse-checkout.c | 3 +--\n>  1 file changed, 1 insertion(+), 2 deletions(-)\n\nAn exact copy of this patch is already in 'next' since Feb 17th, if\nI am not mistaken.\n\n> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\n> index cccf630331..34e965bfa6 100644\n> --- a/builtin/sparse-checkout.c\n> +++ b/builtin/sparse-checkout.c\n> @@ -94,8 +94,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,\n>  \t\t\tstring_list_append(&sl, pe->pattern + 1);\n>  \t\t}\n>  \n> -\t\tstring_list_sort(&sl);\n> -\t\tstring_list_remove_duplicates(&sl, 0);\n> +\t\tstring_list_sort_u(&sl, 0);\n>  \n>  \t\tfor (i = 0; i < sl.nr; i++) {\n>  \t\t\tquote_c_style(sl.items[i].string, NULL, stdout, 0);\n"},{"id":"536629","messageId":"xmqqwm05qsei.fsf@gitster.g","threadId":"64983","inReplyTo":"20260221162359.43336-2-amishhhaaaa@gmail.com","subject":"Re: [PATCH v3 2/2] help: cleanup the contruction of keys_uniq","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-22T05:05:57Z","receivedAt":"2026-02-22T05:06:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n\n> From: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n>\n> +static void show_config_sections(struct string_list *keys)\n> +{\n> ...\n> +}\n> +\n> +static void show_config_vars(struct string_list *keys)\n> +{\n> ...\n> +}\n\nThe striking similarity of the body of the loops in these two\nfunctions bothered me enough to try writing this; the result does\nnot look too bad, I think.\n\nBy the way, I'd really prefer to see contributors *NOT* to use\nundeliverable and/or bouncing e-mail addresses when working on this\nproject, as I'd always have to edit the Cc: list to avoid getting\nbounces.\n\nThanks.\n\n builtin/help.c | 72 ++++++++++++++++++++++------------------------------------\n 1 file changed, 27 insertions(+), 45 deletions(-)\n\ndiff --git c/builtin/help.c w/builtin/help.c\nindex b70de09864..bc5c5a556c 100644\n--- c/builtin/help.c\n+++ w/builtin/help.c\n@@ -120,36 +120,37 @@ static void show_config_human(struct string_list *keys)\n \t}\n }\n \n-static void show_config_sections(struct string_list *keys)\n+static void grab_leading_part(struct string_list *keys, const char *var, int use_dot)\n {\n-\tstruct string_list keys_uniq = STRING_LIST_INIT_DUP;\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tstruct string_list_item *item;\n+\tconst char *cut = NULL;\n \n-\tfor (size_t i = 0; i < keys->nr; i++) {\n-\t\tconst char *var = keys->items[i].string;\n-\t\tconst char *dot = strchr(var, '.');\n-\t\tconst char *wildcard = strchr(var, '*');\n-\t\tconst char *tag = strchr(var, '<');\n-\t\tconst char *cut;\n-\n-\t\tif (dot)\n-\t\t\tcut = dot;\n-\t\telse if (wildcard && tag)\n-\t\t\tcut = wildcard < tag ? wildcard : tag;\n-\t\telse if (wildcard)\n-\t\t\tcut = wildcard;\n-\t\telse if (tag)\n-\t\t\tcut = tag;\n-\t\telse {\n-\t\t\tstring_list_append(&keys_uniq, var);\n-\t\t\tcontinue;\n-\t\t}\n+\tif (use_dot)\n+\t\tcut = strchr(var, use_dot);\n \n+\tif (!cut) {\n+\t\tsize_t prefix_len = strcspn(var, \"*<\");\n+\t\tif (var[prefix_len])\n+\t\t\tcut = var + prefix_len;\n+\t}\n+\n+\tif (!cut)\n+\t\tstring_list_append(keys, var);\n+\telse {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n \t\tstrbuf_add(&sb, var, cut - var);\n-\t\tstring_list_append(&keys_uniq, sb.buf);\n+\t\tstring_list_append(keys, sb.buf);\n \t\tstrbuf_release(&sb);\n \t}\n+}\n+\n+static void show_config_sections(struct string_list *keys)\n+{\n+\tstruct string_list keys_uniq = STRING_LIST_INIT_DUP;\n+\tstruct string_list_item *item;\n+\n+\tfor (size_t i = 0; i < keys->nr; i++)\n+\t\tgrab_leading_part(&keys_uniq, keys->items[i].string, '.');\n+\n \tstring_list_sort_u(&keys_uniq, 0);\n \tfor_each_string_list_item(item, &keys_uniq)\n \t\tputs(item->string);\n@@ -159,30 +160,11 @@ static void show_config_sections(struct string_list *keys)\n static void show_config_vars(struct string_list *keys)\n {\n \tstruct string_list keys_uniq = STRING_LIST_INIT_DUP;\n-\tstruct strbuf sb = STRBUF_INIT;\n \tstruct string_list_item *item;\n \n-\tfor (size_t i = 0; i < keys->nr; i++) {\n-\t\tconst char *var = keys->items[i].string;\n-\t\tconst char *wildcard = strchr(var, '*');\n-\t\tconst char *tag = strchr(var, '<');\n-\t\tconst char *cut;\n-\n-\t\tif (wildcard && tag)\n-\t\t\tcut = wildcard < tag ? wildcard : tag;\n-\t\telse if (wildcard)\n-\t\t\tcut = wildcard;\n-\t\telse if (tag)\n-\t\t\tcut = tag;\n-\t\telse {\n-\t\t\tstring_list_append(&keys_uniq, var);\n-\t\t\tcontinue;\n-\t\t}\n+\tfor (size_t i = 0; i < keys->nr; i++)\n+\t\tgrab_leading_part(&keys_uniq, keys->items[i].string, '\\0');\n \n-\t\tstrbuf_add(&sb, var, cut - var);\n-\t\tstring_list_append(&keys_uniq, sb.buf);\n-\t\tstrbuf_release(&sb);\n-\t}\n \tstring_list_sort_u(&keys_uniq, 0);\n \tfor_each_string_list_item(item, &keys_uniq)\n \t\tputs(item->string);\n"},{"id":"536639","messageId":"CAPvEtrfmgq8f2z7tAvR-oCEYoiG2B+Pj9EqjUsKuewnO73tVPg@mail.gmail.com","threadId":"64983","inReplyTo":"xmqqwm05qsei.fsf@gitster.g","subject":"Re: [PATCH v3 2/2] help: cleanup the contruction of keys_uniq","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-02-22T09:47:19Z","receivedAt":"2026-02-22T09:47:30Z","isPatch":true,"sender":{"key":"amishhhaaaa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/136238836?v=4"},"body":">\n> The striking similarity of the body of the loops in these two\n> functions bothered me enough to try writing this; the result does\n> not look too bad, I think.\n\n\nAgreed, I was also not very happy with the similarity present at these\ntwo places,\nespecially the wildcard and tag part, tried to convulse them into something\nsingular. It again started to look like the original so ultimately\nkept it like this.\n\n>\n> By the way, I'd really prefer to see contributors *NOT* to use\n> undeliverable and/or bouncing e-mail addresses when working on this\n> project, as I'd always have to edit the Cc: list to avoid getting\n> bounces.\n>\n> Thanks.\n>\n\nThanks, I will take care.\n"},{"id":"537206","messageId":"xmqqjyvz4foj.fsf@gitster.g","threadId":"64983","inReplyTo":"CAPvEtrfmgq8f2z7tAvR-oCEYoiG2B+Pj9EqjUsKuewnO73tVPg@mail.gmail.com","subject":"Re: [PATCH v3 2/2] help: cleanup the contruction of keys_uniq","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-26T16:45:16Z","receivedAt":"2026-02-26T16:45:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n\n>>\n>> The striking similarity of the body of the loops in these two\n>> functions bothered me enough to try writing this; the result does\n>> not look too bad, I think.\n>\n>\n> Agreed, I was also not very happy with the similarity present at these\n> two places,\n> especially the wildcard and tag part, tried to convulse them into something\n> singular. It again started to look like the original so ultimately\n> kept it like this.\n>\n>>\n>> By the way, I'd really prefer to see contributors *NOT* to use\n>> undeliverable and/or bouncing e-mail addresses when working on this\n>> project, as I'd always have to edit the Cc: list to avoid getting\n>> bounces.\n>>\n>> Thanks.\n>>\n>\n> Thanks, I will take care.\n\nThanks.\n"},{"id":"537386","messageId":"20260228104654.80831-1-amishhhaaaa@gmail.com","threadId":"64983","inReplyTo":"20260212041017.91370-1-amishhhaaaa@gmail.com","subject":"[PATCH v4 0/1] Make keys_uniq stop depending on sort of keys_uniq","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-02-28T10:46:53Z","receivedAt":"2026-02-28T10:47:25Z","isPatch":true,"sender":{"key":"amishhhaaaa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/136238836?v=4"},"body":"While its problematic for keys_uniq to depend on sort operation on keys,\nreproduction of the issue as a test is complex, when we add something to the\ndocumentation we will need to re-build git so the array(config-list.h)\nis also rebuilt with our new test case so the functions which fail on the test\ncan catch it during runtime.\n\nThe details for the test are as follows:\nthe case[0] and steps to reproduce[1].\n[0] https://lore.kernel.org/git/CAPvEtrfEZXHxcDf=z60ODfUA8cS81rhF1y7KEZApEBby7aCa1A@mail.gmail.com/\n[1] https://lore.kernel.org/git/20260212041017.91370-1-amishhhaaaa@gmail.com/T/#m64880c5cd0d36e35bc78692757cf206b13496aea\n\nCommunity help is appreciated as i tried various ways to reproduce the issue\nin a test, including cat to config-list.h, cat to documentation config,\napplied patch\nhttps://lore.kernel.org/git/9cdcc9de04f0f8fff657f0474b31c063466ed808.1771280837.git.ben.knoble+github@gmail.com/T/#me826da3b6a128e1ceb7215d64328b7d6aa2b211e\nas well, but unfortunately, I couldn't get the test in config-list during\ntest runtime as that array is baked into the binary and needs rebuild to show.\n\nThanks.\n\nAmisha Chhajed (1):\n  help: cleanup the contruction of keys_uniq\n\n builtin/help.c  | 84 ++++++++++++++++++++++++++++++-------------------\n t/t0012-help.sh | 26 +++++++--------\n 2 files changed, 65 insertions(+), 45 deletions(-)\n\n-- \n2.52.0\n\n"},{"id":"537387","messageId":"20260228104654.80831-2-amishhhaaaa@gmail.com","threadId":"64983","inReplyTo":"20260228104654.80831-1-amishhhaaaa@gmail.com","subject":"[PATCH v4 1/1] help: cleanup the contruction of keys_uniq","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-02-28T10:46:54Z","receivedAt":"2026-02-28T10:47:30Z","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.\ndedent sort -u and sed in tests and replace grep with sed.\n\nSigned-off-by: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>\n---\n builtin/help.c  | 84 ++++++++++++++++++++++++++++++-------------------\n t/t0012-help.sh | 26 +++++++--------\n 2 files changed, 65 insertions(+), 45 deletions(-)\n\ndiff --git a/builtin/help.c b/builtin/help.c\nindex c09cbc8912..3658836d23 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@@ -156,50 +199,27 @@ static void list_config_help(enum show_config_type type)\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..03104b3bf4 100755\n--- a/t/t0012-help.sh\n+++ b/t/t0012-help.sh\n@@ -141,20 +141,20 @@ 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+\tsed \\\n+\t\t-e \"/^[^.]*\\.[^.]*$/d\" \\\n+\t\t-e \"/^[^.]*\\.[^.]*\\.[^.]*$/d\" \\\n \t\thelp.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 -n \\\n+\t     -e \"/^[^.]*\\.[^.]*$/p\" \\\n+\t     -e \"/^[^.]*\\.[^.]*\\.[^.]*$/p\" human |\n+\tsed -e \"s/\\*.*//\" -e \"s/<.*//\" |\n+\tsort -u >human.munged &&\n \n \tgit help --config-for-completion >vars &&\n \ttest_cmp human.munged vars\n@@ -162,11 +162,11 @@ 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+\tsed -n \\\n+\t     -e \"/^[^.]*\\.[^.]*$/p\" \\\n+\t     -e \"/^[^.]*\\.[^.]*\\.[^.]*$/p\" human |\n+\tsed -e \"s/\\..*//\" |\n+\tsort -u >human.munged &&\n \n \tgit help --config-sections-for-completion >sections &&\n \ttest_cmp human.munged sections\n-- \n2.52.0\n\n"},{"id":"537388","messageId":"CAPvEtrf_m1Uae27Z9ZKsSJsu=_HAeT8fMO80cnVGc4dfVtrTBQ@mail.gmail.com","threadId":"64983","inReplyTo":"xmqqjyvz4foj.fsf@gitster.g","subject":"Re: [PATCH v3 2/2] help: cleanup the contruction of keys_uniq","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-02-28T10:51:15Z","receivedAt":"2026-02-28T10:51:28Z","isPatch":true,"sender":{"key":"amishhhaaaa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/136238836?v=4"},"body":"Incredibly sorry for the bouncing mail once again, I will fix it locally.\n\n\nOn Thu, 26 Feb 2026 at 22:15, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n>\n> >>\n> >> The striking similarity of the body of the loops in these two\n> >> functions bothered me enough to try writing this; the result does\n> >> not look too bad, I think.\n> >\n> >\n> > Agreed, I was also not very happy with the similarity present at these\n> > two places,\n> > especially the wildcard and tag part, tried to convulse them into something\n> > singular. It again started to look like the original so ultimately\n> > kept it like this.\n> >\n> >>\n> >> By the way, I'd really prefer to see contributors *NOT* to use\n> >> undeliverable and/or bouncing e-mail addresses when working on this\n> >> project, as I'd always have to edit the Cc: list to avoid getting\n> >> bounces.\n> >>\n> >> Thanks.\n> >>\n> >\n> > Thanks, I will take care.\n>\n> Thanks.\n\n\n\n-- \nThanks,\nAmisha\n"},{"id":"537539","messageId":"xmqqwlzu43rh.fsf@gitster.g","threadId":"64983","inReplyTo":"20260228104654.80831-2-amishhhaaaa@gmail.com","subject":"Re: [PATCH v4 1/1] help: cleanup the contruction of keys_uniq","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-02T16:04:02Z","receivedAt":"2026-03-02T16:04:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n\n> diff --git a/t/t0012-help.sh b/t/t0012-help.sh\n> index d3a0967e9d..03104b3bf4 100755\n> --- a/t/t0012-help.sh\n> +++ b/t/t0012-help.sh\n> @@ -141,20 +141,20 @@ 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> +\tsed \\\n> +\t\t-e \"/^[^.]*\\.[^.]*$/d\" \\\n> +\t\t-e \"/^[^.]*\\.[^.]*\\.[^.]*$/d\" \\\n>  \t\thelp.output >actual &&\n\nWe used to require at least one non-dot byte between each dot in the\noriginal, but now we do not.  Is this change in semantics intended?\n\nYou could fix it with \"sed -E\" and keeping the ERE in the original,\nI guess?\n\nIt was in a distant past when I tried benchmarking them for the last\ntime, but I recall \"sed\" was a lot slower than \"grep\" on a \"match\nand print\" job that \"grep\" could be an alternative.  So I am not\nsure what the point of the change in this hunk is.\n\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 -n \\\n> +\t     -e \"/^[^.]*\\.[^.]*$/p\" \\\n> +\t     -e \"/^[^.]*\\.[^.]*\\.[^.]*$/p\" human |\n> +\tsed -e \"s/\\*.*//\" -e \"s/<.*//\" |\n\nDitto.\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> +\tsed -n \\\n> +\t     -e \"/^[^.]*\\.[^.]*$/p\" \\\n> +\t     -e \"/^[^.]*\\.[^.]*\\.[^.]*$/p\" human |\n> +\tsed -e \"s/\\..*//\" |\n\nDitto.\n\nJust like piping \"grep\" output to \"sed\" is an anti-pattern, piping\n\"sed\" output to an invocation of \"sed\" is often an anti-pattern.\n\nPerhaps something like this would replace the original \"grep | sed\"\npipeline?\n\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 |\n\tsort -u\n\n\n"},{"id":"537540","messageId":"xmqqseai43n8.fsf@gitster.g","threadId":"64983","inReplyTo":"CAPvEtrf_m1Uae27Z9ZKsSJsu=_HAeT8fMO80cnVGc4dfVtrTBQ@mail.gmail.com","subject":"Re: [PATCH v3 2/2] help: cleanup the contruction of keys_uniq","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-02T16:06:35Z","receivedAt":"2026-03-02T16:06:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n\n> Incredibly sorry for the bouncing mail once again, I will fix it locally.\n\nThanks for noticing.  Your v4 has the same address.\n"},{"id":"538662","messageId":"20260311192453.62213-1-amishhhaaaa@gmail.com","threadId":"64983","inReplyTo":"20260212041017.91370-1-amishhhaaaa@gmail.com","subject":"[PATCH v5] help: cleanup the contruction of keys_uniq","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-03-11T19:24:53Z","receivedAt":"2026-03-11T19:25:06Z","isPatch":true,"sender":{"key":"amishhhaaaa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/136238836?v=4"},"body":"construction 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"},{"id":"538671","messageId":"xmqq7brino7h.fsf@gitster.g","threadId":"64983","inReplyTo":"20260311192453.62213-1-amishhhaaaa@gmail.com","subject":"Re: [PATCH v5] help: cleanup the contruction of keys_uniq","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-11T19:46:58Z","receivedAt":"2026-03-11T19:47:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n\n> @@ -162,14 +165,16 @@ test_expect_success 'git help --config-for-completion' '\n> ...\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\nThe blank has a HT, which is a trailing whitespace.  No need to\nresend only to correct this, as \"git am\" on my end cleaned it up\nalready while queuing.\n"},{"id":"538673","messageId":"CAPvEtrf7gqyQYMcsii===kXY5Vut0EC_VsJ=xWUKNrq6YmA=nA@mail.gmail.com","threadId":"64983","inReplyTo":"xmqqwlzu43rh.fsf@gitster.g","subject":"Re: [PATCH v4 1/1] help: cleanup the contruction of keys_uniq","fromName":"Amisha Chhajed","fromEmail":"amishhhaaaa@gmail.com","sentAt":"2026-03-11T19:48:49Z","receivedAt":"2026-03-11T19:49:01Z","isPatch":true,"sender":{"key":"amishhhaaaa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/136238836?v=4"},"body":"> Perhaps something like this would replace the original \"grep | sed\"\n> pipeline?\n>\n>         sed -E -e \"\n>                 /^[^.]+\\.[^.]+$/b out\n>                 /^[^.]+\\.[^.]+\\.[^.]+$/b out\n>                 d\n>                 : out\n>                 s/\\..*//\n>         \" human |\n>         sort -u\n>\n>\n\nThank you for pointing me in the right direction!\n\n-- \nThanks,\nAmisha\n"},{"id":"538679","messageId":"xmqqfr66m5qj.fsf@gitster.g","threadId":"64983","inReplyTo":"CAPvEtrf7gqyQYMcsii===kXY5Vut0EC_VsJ=xWUKNrq6YmA=nA@mail.gmail.com","subject":"Re: [PATCH v4 1/1] help: cleanup the contruction of keys_uniq","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-11T21:11:16Z","receivedAt":"2026-03-11T21:11:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Amisha Chhajed <amishhhaaaa@gmail.com> writes:\n\n>> Perhaps something like this would replace the original \"grep | sed\"\n>> pipeline?\n>>\n>>         sed -E -e \"\n>>                 /^[^.]+\\.[^.]+$/b out\n>>                 /^[^.]+\\.[^.]+\\.[^.]+$/b out\n>>                 d\n>>                 : out\n>>                 s/\\..*//\n>>         \" human |\n>>         sort -u\n>>\n>>\n>\n> Thank you for pointing me in the right direction!\n\nThis unfortunately runs afoul of t/check-non-portable-shell.pl aka\n\"make -C t test-lint\".  The particular rule was introduced in early\n2019 with e62e225f (test-lint: only use only sed [-n] [-e command]\n[-f command_file], 2019-01-20).  See the attached patch at the end.\n\nIt does cite the then-current POSIX.1 (Issue 7, 2018 edition) but\nthe latest edition (Issue 8) documents \"-E\" as an option to use ERE\n\nI wonder if the situation has improved in the past 7 years.\n\nWe seem to have started using \"sed -E\" without anybody complaining\nin 2022, with 461fec41 (bisect run: keep some of the post-v2.30.0\noutput, 2022-11-10).  It was hidden because the 'E' was squished\nwith another single letter option.\n\nt/t6030-bisect-porcelain.sh:\tsed -En 's/.*(bisect.*code) (-?[0-9]+) (from.*)/\\1 -1 \\3/p' err >actual &&\n\nSo I think of no strong reason to reject another new use of \"sed\n-E\".  I am tempted to revert e62e225f (test-lint: only use only sed\n[-n] [-e command] [-f command_file], 2019-01-20), whose intention\nwas to reject anything other than \"-[efn]\", to its previous form\nwhich rejected only \"sed -i\".\n\nAlternatively, I would of course welcome volunteers to revamp the\ncheck-non-portable-shell.pl script to make the pattern more robust,\nand then add 'E' to the set of allowed options, but I somehow do not\nthink that is a good use of our engineering resources.\n\nFor example, in addition to the escape we see in t6030 above, the\ncurrent pattern would not catch use of -E if it is written this way:\n\n\tsed \"-E\" -e \"\n\t\t...\n\t\" human |\n\tsort -u\n\nor\n\n\tsed \\\n\t\t-E -e \"\n\t\t...\n\t\" human |\n\tsort -u\n\nand million other ways to subvert the simple-minded pattern-match\nbased check.\n\nOpinions?\n\n\ncommit e62e225ffb589e59c4f64d90b0a393aa6a0a5ace\nAuthor: Torsten Bögershausen <tboegi@web.de>\nDate:   Sun Jan 20 08:53:50 2019 +0100\n\n    test-lint: only use only sed [-n] [-e command] [-f command_file]\n    \n    From `man sed` (on a Mac OS X box):\n    The -E, -a and -i options are non-standard FreeBSD extensions and may not be available\n    on other operating systems.\n    \n    From `man sed` on a Linux box:\n    REGULAR EXPRESSIONS\n           POSIX.2 BREs should be supported, but they aren't completely because of\n           performance problems.  The \\n sequence in a regular expression matches the newline\n           character,  and  similarly  for \\a, \\t, and other sequences.\n           The -E option switches to using extended regular expressions instead; the -E option\n           has been supported for years by GNU sed, and is now included in POSIX.\n    \n    Well, there are still a lot of systems out there, which don't support it.\n    Beside that, IEEE Std 1003.1TM-2017, see\n    http://pubs.opengroup.org/onlinepubs/9699919799/\n    does not mention -E either.\n    \n    To be on the safe side, don't allow -E (or -r, which is GNU).\n    Change check-non-portable-shell.pl to only accept the portable options:\n    sed [-n] [-e command] [-f command_file]\n    \n    Reported-by: SZEDER Gábor <szeder.dev@gmail.com>\n    Helped-by: Eric Sunshine <sunshine@sunshineco.com>\n    Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n    Signed-off-by: Torsten Bögershausen <tboegi@web.de>\n    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex b45bdac688..f0edcf8eb0 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -35,7 +35,7 @@ sub err {\n \t\tchomp;\n \t}\n \n-\t/\\bsed\\s+-i/ and err 'sed -i is not portable';\n+\t/\\bsed\\s+-[^efn]\\s+/ and err 'sed option not portable (use only -n, -e, -f)';\n \t/\\becho\\s+-[neE]/ and err 'echo with option is not portable (use printf)';\n \t/^\\s*declare\\s+/ and err 'arrays/declare not portable';\n \t/^\\s*[^#]\\s*which\\s/ and err 'which is not portable (use type)';\n"},{"id":"538683","messageId":"CAPig+cQ+HLjBjtGA9s_ZYYWNjRj_Bax5CkJFa98a-z=LoyEFoQ@mail.gmail.com","threadId":"64983","inReplyTo":"xmqqfr66m5qj.fsf@gitster.g","subject":"Re: [PATCH v4 1/1] help: cleanup the contruction of keys_uniq","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2026-03-11T21:39:39Z","receivedAt":"2026-03-11T21:39:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Mar 11, 2026 at 5:11 PM Junio C Hamano <gitster@pobox.com> wrote:\n> For example, in addition to the escape we see in t6030 above, the\n> current pattern would not catch use of -E if it is written this way:\n>\n>         sed \"-E\" -e \"\n>                 ...\n>         \" human |\n>         sort -u\n\nSeems unlikely to arise in practice.\n\n> or\n>\n>         sed \\\n>                 -E -e \"\n>                 ...\n>         \" human |\n>         sort -u\n\nFor what it's worth, line folding capability was added to\ncheck-non-portable-shell.pl by a0a630192d (t/check-non-portable-shell:\ndetect \"FOO=bar shell_func\", 2018-07-13), so it does correctly detect\nthe errant -E in this example.\n\n> and million other ways to subvert the simple-minded pattern-match\n> based check.\n\nTrue, for sure.\n"},{"id":"538686","messageId":"xmqqwlzikpbz.fsf@gitster.g","threadId":"64983","inReplyTo":"CAPig+cQ+HLjBjtGA9s_ZYYWNjRj_Bax5CkJFa98a-z=LoyEFoQ@mail.gmail.com","subject":"Re: [PATCH v4 1/1] help: cleanup the contruction of keys_uniq","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-11T21:50:56Z","receivedAt":"2026-03-11T21:50:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>>         sed \\\n>>                 -E -e \"\n>>                 ...\n>>         \" human |\n>>         sort -u\n>\n> For what it's worth, line folding capability was added to\n> check-non-portable-shell.pl by a0a630192d (t/check-non-portable-shell:\n> detect \"FOO=bar shell_func\", 2018-07-13), so it does correctly detect\n> the errant -E in this example.\n\nAh, thanks for correcting me.\n\nBut \"sed -n -i -e '/.../p'\" would not catch \"-i\", and that is not\nall that unlikely, I suspect.\n"},{"id":"538687","messageId":"CAPig+cS4vUDu0j5w3XvgdCXTV1bnwqeoGN3MRtmjvYsaMwsp6g@mail.gmail.com","threadId":"64983","inReplyTo":"xmqqwlzikpbz.fsf@gitster.g","subject":"Re: [PATCH v4 1/1] help: cleanup the contruction of keys_uniq","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2026-03-11T21:54:53Z","receivedAt":"2026-03-11T21:55:05Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Mar 11, 2026 at 5:51 PM Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n>\n> >>         sed \\\n> >>                 -E -e \"\n> >>                 ...\n> >>         \" human |\n> >>         sort -u\n> >\n> > For what it's worth, line folding capability was added to\n> > check-non-portable-shell.pl by a0a630192d (t/check-non-portable-shell:\n> > detect \"FOO=bar shell_func\", 2018-07-13), so it does correctly detect\n> > the errant -E in this example.\n>\n> Ah, thanks for correcting me.\n>\n> But \"sed -n -i -e '/.../p'\" would not catch \"-i\", and that is not\n> all that unlikely, I suspect.\n\nCorrect. By only looking at the very first option following the\ncommand name (`sed`), the checking performed by\ncheck-non-portable-shell.pl is very weak indeed.\n"}]}