public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5] help: cleanup the contruction of keys_uniq
@ 2026-03-11 19:21 Amisha Chhajed
  0 siblings, 0 replies; 3+ messages in thread
From: Amisha Chhajed @ 2026-03-11 19:21 UTC (permalink / raw)
  To: git; +Cc: gitster, ps, r.siddharth.shrimali, amishhhaaaa, Amisha Chhajed

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

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>
---
 builtin/help.c  | 91 ++++++++++++++++++++++++++++++-------------------
 t/t0012-help.sh | 39 ++++++++++++---------
 2 files changed, 78 insertions(+), 52 deletions(-)

diff --git a/builtin/help.c b/builtin/help.c
index c09cbc8912..daadc61c70 100644
--- 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 d3a0967e9d..40b2d656a5 100755
--- 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 () {
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* [PATCH 0/2] clean leftover calls to string_list_remove_duplicates
@ 2026-02-12  4:10 Amisha Chhajed
  2026-03-11 19:24 ` [PATCH v5] help: cleanup the contruction of keys_uniq Amisha Chhajed
  0 siblings, 1 reply; 3+ messages in thread
From: Amisha Chhajed @ 2026-02-12  4:10 UTC (permalink / raw)
  To: git; +Cc: gitster, stolee, peff, avarab, Amisha Chhajed

replace calls to string_list_remove_duplicates with string_list_sort_u.

sorted behavior of &keys_uniq depends on the call to string_list_sort on
&keys before &keys is processed to form &keys_uniq, this introduces an
edge case where &keys_uniq will not be sorted as expected, follow [1]
to reproduce.
add string_list_sort_u to &keys_uniq to fix this edge case and ensure
future enhancements to the processing logic does not introduce regressions.

[1]
run command:
cat <<'EOF' >> Documentation/config/add.adoc
aa*.b::
aa.b::
EOF
from git/ and then make test, this sees one failure when the test compared
actual output with sort -u output.

Amisha Chhajed (2):
  sparse-checkout: use string_list_sort_u
  help: ensure &keys_uniq follows sort -u

 builtin/help.c            | 2 +-
 builtin/sparse-checkout.c | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

-- 
2.52.0


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-03-11 19:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-11 19:21 [PATCH v5] help: cleanup the contruction of keys_uniq Amisha Chhajed
  -- strict thread matches above, loose matches on Subject: below --
2026-02-12  4:10 [PATCH 0/2] clean leftover calls to string_list_remove_duplicates Amisha Chhajed
2026-03-11 19:24 ` [PATCH v5] help: cleanup the contruction of keys_uniq Amisha Chhajed
2026-03-11 19:46   ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox