On Tue, Nov 14, 2023 at 10:08:46AM +0900, Junio C Hamano wrote:

SZEDER Gábor [off-list ref] writes:

quoted

+# Lists all subsections in the given section which contain the given
+# config variable, with the section and variable names removed.
+__git_get_config_subsections ()
+{
+	local section="$1" var="$2" i IFS=$'\n'
+	for i in $(__git config --name-only --get-regexp "^$section\..*\.$var$"); do
+		i=${i#$section.}
+		i=${i%.$var}

As this script is allowed bash-isms, I wondered if we can use
a single pattern substitution instead of two remove pre/suffix
pattern substitution, but I guess it would not work, and the above
is perfectly readable.

Yeah, I don't think it's possible to remove the prefix and suffix with
Bash builtins in a single operation.

quoted

+		echo "$i"

As the subsection is designed to contain unbounded set of end-user
controlled names, we probably should do

		printf "%s\n" "$i"

instead to protect us from interesting names (e.g. ones that begin
with a dash).

Indeed, will do.

quoted

+	done
+}

Interesting to see that we do not need to bother deduplicating the
output from here.

Bash will sort and deduplicate the completion words anyway, so we
don't have to.  Sometimes we do deduplicate them, though, but either
to make testing easier or for performance reasons; in this case
neither of them applies.

quoted

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index a7c3b4eb63..11ed83d0ed 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh

@@ -2130,6 +2130,19 @@ test_expect_success '__git_get_config_variables' '
 	test_cmp expect actual
 '
 
+test_expect_success '__git_get_config_subsections' '
+	cat >expect <<-\EOF &&
+	subsection-1
+	SubSection-2
+	sub.section.3
+	EOF
+	test_config interesting.subsection-1.name good &&
+	test_config Interesting.SubSection-2.Name good &&
+	test_config interesting.sub.section.3.name good &&
+	__git_get_config_subsections interesting name >actual &&
+	test_cmp expect actual
+'

Good to see an uppercase character is used here ;-).

That's just for good measure, but not really necessasry here, as that
primarily tests that 'git config' lists the section and variable names
(but not the subsection!) normalized to lowercase, no matter what
CaMeLCase the user might have written them.