[PATCH v2 2/2] help: cleanup the contruction of keys_uniq

[Amisha Chhajed <amishhhaaaa@gmail.com>]

From: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>

uniqueness property of keys_uniq depends on the sort operation executed
for keys, sorted property of keys does not gurantee sorted property of
keys_uniq due to processing keys, this might also introduce regressions
in future when the logic of forming keys_uniq from keys is changed.

add string_list_sort_u operation for keys_uniq and refactor the
processing code to simplify it, add test that demonstrates that sorted
property of keys does not gurantee sorted property of keys_uniq.

Signed-off-by: Amisha Chhajed <136238836+amishhaa@users.noreply.github.com>
---
 builtin/help.c  | 71 +++++++++++++++++++++++++------------------------
 t/t0012-help.sh | 18 +++++++++++++
 2 files changed, 54 insertions(+), 35 deletions(-)

diff --git a/builtin/help.c b/builtin/help.c
index c09cbc8912..c278d7ffcb 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -156,47 +156,48 @@ static void list_config_help(enum show_config_type type)
 			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:
+	if (type == SHOW_CONFIG_HUMAN) {
+		string_list_sort(&keys);
+		for (size_t i = 0; i < keys.nr; i++) {
+			const char *var = keys.items[i].string;
 			puts(var);
-			continue;
-		case SHOW_CONFIG_SECTIONS:
-			dot = strchr(var, '.');
-			break;
-		case SHOW_CONFIG_VARS:
-			break;
-		}
-		wildcard = strchr(var, '*');
-		tag = strchr(var, '<');
-
-		if (!dot && !wildcard && !tag) {
-			string_list_append(&keys_uniq, var);
-			continue;
 		}
+	}
+	else{
+		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;
+
+			if (type == SHOW_CONFIG_SECTIONS) {
+				dot = strchr(var, '.');
+			}
+			wildcard = strchr(var, '*');
+			tag = strchr(var, '<');
 
-		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);
+			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);
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index d3a0967e9d..0dbe6dd46f 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -160,6 +160,24 @@ test_expect_success 'git help --config-for-completion' '
 	test_cmp human.munged vars
 '
 
+test_expect_success 'git help --config-for-completion' '
+	file="$GIT_SOURCE_DIR/Documentation/config/add.adoc" &&
+	test_when_finished "git -C \"$GIT_SOURCE_DIR\" checkout -- Documentation/config/add.adoc" &&
+	cat <<-\EOF >>"$file" &&
+	aa*.b::
+	aa.b::
+	EOF
+	git help -c >human &&
+	grep -E \
+	     -e "^[^.]+\.[^.]+$" \
+	     -e "^[^.]+\.[^.]+\.[^.]+$" human |
+	     sed -e "s/\*.*//" -e "s/<.*//" |
+	     sort -u >human.munged &&
+
+	git help --config-for-completion >vars &&
+	test_cmp human.munged vars
+'
+
 test_expect_success 'git help --config-sections-for-completion' '
 	git help -c >human &&
 	grep -E \
-- 
2.52.0