Subject: Re: [PATCH v2 3/5] completion: add and use __git_compute_first_level_config_vars_for_section

From: Junio C Hamano <[email-masked]>
Date: Sat, 10 Feb 2024 09:15:43 -0800

Philippe Blain <[email-masked]> writes:

>>> +	__git_compute_config_vars
>>> +	local this_section="__git_first_level_config_vars_for_section_${section}"
>>> +	test -n "${!this_section}" ||
>>> +	printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')"
>>> +}

A silly question (primarily because I do not much use the indirect
reference construct ${!name}).  Does the assignment with printf need
to spell out the long variable name with "_${section}"?  Can it be

    printf -v "$this_section" ...

instead, as we already have the short-hand for it?

> finds also others. I think the idea is to cache these lists to avoid 
> computing them everytime they are needed (probably most useful on Windows 
> where process creation is longer). I'll mention that in the 
> commit message.

Yup, as long as the contents of the list stays stable (e.g., list of
Git subcommands, list of options a Git subcommand takes, list of
configuration variable names that do not have end-user customization
part, etc.), it is a viable optimization technique.  The available
<slot> for color.branch.<slot> and color.diff.<slot> do not change
(unless you talk about new version of Git adding support for more
slots) and is a good idea to cache.  remote.<name>.url takes its
<name> component out of an unbound set of end-user controlled names,
so unless we somehow have a method to invalidate cached values, the
list can go stale as remotes are added and removed.

Thanks.