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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 10, 2024, 17:15 UTC
Message-ID: <xmqqwmrcb5q8.fsf@gitster.g>
In-Reply-To: <e8642ad8-bdc9-00d6-39b5-81dd399e60ec@gmail.com>

Philippe Blain <levraiphilippeblain@gmail.com> writes:

Show 5 quoted lines

>>> +	__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.