| Message ID | 20231113222528.62771-1-szeder.dev@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | completion: add and use the __git_get_config_subsection helper function | expand |
SZEDER Gábor <szeder.dev@gmail.com> writes: > +# 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. > + 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). > + done > +} Interesting to see that we do not need to bother deduplicating the output from here. > 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 ;-). Thanks.
On Tue, Nov 14, 2023 at 10:08:46AM +0900, Junio C Hamano wrote: > SZEDER Gábor <szeder.dev@gmail.com> writes: > > > +# 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. > > + 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. > > + 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. > > 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.
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 13a39ebd2e..34bbb66f85 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1148,6 +1148,18 @@ __git_get_config_variables () done } +# 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} + echo "$i" + done +} + __git_pretty_aliases () { __git_get_config_variables "pretty" @@ -1681,7 +1693,7 @@ __git_untracked_file_modes="all no normal" __git_trailer_tokens () { - __git config --name-only --get-regexp '^trailer\..*\.key$' | cut -d. -f 2- | rev | cut -d. -f2- | rev + __git_get_config_subsections trailer key } _git_commit () 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 +' + test_expect_success '__git_pretty_aliases' ' cat >expect <<-EOF && author
Our Bash completion script recently learned to complete configured trailer key aliases for 'git config --trailer=<TAB>', but the helper function extracting the key alias from 'git config's output, i.e. the subsection from 'trailer.*.key', ended up more complex than necessary, with considerable overhead from executing four external processes in a pipe. Replace those commands in the pipe with a simple shell loop using only a pair of parameter expansions and a builtin 'echo', which is easier to understand and should perform better (I assume that users don't have that many subsections in any particular section to make the processing with an external process (let alone four) worth it). And while at it, let's extract this loop into a generic __git_get_config_subsections() helper function, as it might be useful elsewhere in the future as well (at the moment it isn't, AFAICT). Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- contrib/completion/git-completion.bash | 14 +++++++++++++- t/t9902-completion.sh | 13 +++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-)